Skip to content

Guard the UnixTime globals, so JsonWebToken and Security can be used together - #14

Open
mckuipers wants to merge 2 commits into
DataFlex-dev:production/stablefrom
mckuipers:fix/guard-currentunixtime-global-redefinition
Open

Guard the UnixTime globals, so JsonWebToken and Security can be used together#14
mckuipers wants to merge 2 commits into
DataFlex-dev:production/stablefrom
mckuipers:fix/guard-currentunixtime-global-redefinition

Conversation

@mckuipers

Copy link
Copy Markdown

JWT/UnixTime.pkg guards five of its definitions — gVoid, tFileTime, tSystemTime, and
both External_Functions — but not its two global functions. This adds the missing guards.

Two commits, separable: the first fixes a real collision, the second is consistency.

Commit 1 — CurrentUnixTime, which actually clashes

DataFlex-dev/Security ships a byte-identical copy of this file as include/UnixTime.pkg,
also unguarded. DataFlex resolves a Use by path, so to the compiler these are two unrelated
files, and any program containing both gets the global twice:

Error 4390: Illegal method name definition
            Global method GET CURRENTUNIXTIME already defined

That combination is not exotic — it is what the OAuth Server Lib needs. OAuth Server Lib
requires this package, and any OIDC provider offering TOTP as a second factor also wants
cSecureOneTimePassword from Security. The first build that puts both in one program fails,
and the error points into a package-manager-installed folder that cannot be edited, so it reads
like a broken dependency rather than a name collision.

Reproduce by building any program that uses cSecureOneTimePassword and JWT.pkg together.

The matching PR is open on DataFlex-dev/Security. Guarding either copy fixes the collision;
both are being offered one so neither library depends on Use order.

Commit 2 — UnixTimeToDateTime, for consistency

Not needed for the collision above: Security's copy does not define UnixTimeToDateTime, so
that name does not clash between the two libraries today. It makes the file idempotent —
including it a second time by a different path becomes a no-op rather than an error — which is
what the six guards around it already achieve for everything else.

Drop this commit if you would rather change only the name that actually clashes.

DateTimetoUnixTime deliberately gets no guard

It is declared without the Global keyword, and the test below shows it does not collide.

Why Get_<Name> is the right symbol

A Function <Name> Global defines Get_<Name>. The precedent is DataFlex's own
cWorkspace.pkg, which guards Function LastDelimeter Global with #IFDEF Get_LastDelimeter.

I used this file's existing #IFNDEF … #ENDIF style rather than cWorkspace.pkg's
#IFDEF … #ELSE … #ENDIF, to match the guards already here.

Verification

Compiled this file into a program that already contains the whole package, so every global in
it is defined twice by two different paths:

Result
unpatched 4390 at (40,1) GET CURRENTUNIXTIME
commit 1 only 4390 at (59,1) GET UNIXTIMETODATETIME
both commits clean compile

DateTimetoUnixTime was double-included in all three runs and never raised 4390, which is
the basis for leaving it alone. The compiler reports all 4390s in a run rather than stopping at
the first, so its absence is evidence and not an artefact of ordering.

The same build asserts the symbol name is right rather than assuming it:

#IFNDEF Get_CurrentUnixTime
    #ERROR 999 Get_CurrentUnixTime is NOT the symbol
#ENDIF
#IFDEF Get_CurrentUnixTimeNeverDefinedAnywhere
    #ERROR 998 IFDEF matched an undefined symbol
#ENDIF

placed after a known definition. Neither #ERROR fires — the second exists so the first cannot
pass vacuously.

JWT/UnixTime.pkg guards its other five definitions - gVoid, tFileTime,
tSystemTime, and the two External_Functions - but `Function CurrentUnixTime
Global` had no #IFNDEF around it.

DataFlex-dev/Security ships a byte-identical copy of this file as
include/UnixTime.pkg, also unguarded. DataFlex resolves a Use by path, so to
the compiler these are two unrelated files and the global is defined twice:

    Error 4390: Illegal method name definition
                Global method GET CURRENTUNIXTIME already defined

That combination is not exotic: it is what the OAuth Server Lib needs. OAuth
Server Lib requires this package, and any OIDC provider offering TOTP as a
second factor also wants cSecureOneTimePassword from Security. The first
build putting both in one program fails, pointing at a
package-manager-installed file that cannot be edited - so it reads like a
broken dependency rather than a name collision.

Get_CurrentUnixTime is the symbol a `Function ... Global` defines; the
precedent is DataFlex's own cWorkspace.pkg, which guards `Function
LastDelimeter Global` with `#IFDEF Get_LastDelimeter`. Verified by compiling
this file into a program that already defines CurrentUnixTime: 4390 before,
clean after.

Guarding either copy fixes the collision. Both are being offered one, so
neither library depends on Use order.
Not needed for the Security collision in the previous commit - Security's
copy of this file does not define UnixTimeToDateTime, so that name does not
clash between the two libraries today. This is for consistency with the six
guards now around it, and it makes the file idempotent: including it a second
time by a different path becomes a no-op rather than Error 4390.

Demonstrated by compiling this file into a program that already has the whole
package. With only CurrentUnixTime guarded:

    Error 4390: Illegal method name definition
                Global method GET UNIXTIMETODATETIME already defined

With both guarded, it compiles clean.

DateTimetoUnixTime below needs no guard and does not get one: it is declared
without the Global keyword, and the same double-inclusion test shows it does
not collide.

Separable from the previous commit - drop this one if you would rather keep
the change to the single name that actually clashes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant