Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/environment-variable-names.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: Environment variable names

on:
pull_request:
push:
branches:
- main
- rc
workflow_dispatch:

permissions:
contents: read

jobs:
environment-variable-names:
name: No tracked file names a variable starting with a digit or an old name
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
persist-credentials: false

- name: Test the check
shell: pwsh
run: Invoke-Pester -Path ./ci/check-environment-variable-names.Tests.ps1 -CI

- name: Check the names
shell: pwsh
run: ./ci/check-environment-variable-names.ps1
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ java -cp examples/ip-intelligence/console/target/ip-intelligence-java-examples.c
```

The example downloads the Lite file first when it is not there. In place of
the argument, the `51DEGREES_IPI_PATH` environment variable or system property
the argument, the `_51DEGREES_IPI_PATH` environment variable or system property
can give the absolute path of a data file that is already there.

On Java 24 or later, add `--enable-native-access=ALL-UNNAMED` before the class
Expand Down Expand Up @@ -183,7 +183,7 @@ java -cp examples/device-detection/console/target/device-detection-java-examples

It prints the same `Input values:` and `Results:` blocks as the on-premise
example. The IP Intelligence cloud examples read their key from
`51DEGREES_RESOURCE_KEY` first and `TestResourceKey` second, and
`_51DEGREES_RESOURCE_KEY` first and `TestResourceKey` second, and
`examples/ip-intelligence/README.md` gives the keys to create for them.

### Pitfalls
Expand Down
92 changes: 92 additions & 0 deletions ci/check-environment-variable-names.Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Tests for ci/check-environment-variable-names.ps1.
#
# Each case is one line of text and what the check must make of it. The
# digit-initial name is matched in any case, because Windows reads an
# environment variable name in any case, so a lower case old name is still
# the old name there. The old names keep their case, for the reason the check
# gives.

BeforeAll {
$script:Check = Join-Path $PSScriptRoot 'check-environment-variable-names.ps1'
. $script:Check

function Get-Kind([string]$Line) {
@(Find-EnvironmentNameProblem $Line | ForEach-Object { $_.Kind })
}
}

Describe 'Find-EnvironmentNameProblem' {

It 'finds the digit-initial name <Line>' -ForEach @(
@{ Line = 'System.getenv("51DEGREES_IPI_PATH")' }
@{ Line = 'public static final String IPI = "51DEGREES_IPI_PATH";' }
@{ Line = '<DataFile>${51DEGREES_DD_PATH:-lite.hash}</DataFile>' }
@{ Line = "'51DEGREES_DD_PATH' = (Resolve-Path `$file).Path" }
@{ Line = 'System.getenv("51degrees_ipi_path")' }
@{ Line = '$env:51Degrees_Resource_Key = "key"' }
) {
Get-Kind $Line | Should -Contain 'digit'
}

It 'finds the old name <Line>' -ForEach @(
@{ Line = 'System.getenv("RESOURCE_KEY")' }
@{ Line = 'env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY");' }
@{ Line = 'env("_51DEGREES_LICENSE_KEY", "LICENSE_KEY");' }
@{ Line = 'public static final String FOD = "FOD_CLOUD_API_URL";' }
@{ Line = '$env:LICENSE_KEY = $Keys.TestResourceKey' }
@{ Line = '${env:LICENSE_KEY} = $Keys.TestResourceKey' }
@{ Line = '${Env:RESOURCE_KEY}' }
@{ Line = '$ENV:RESOURCE_KEY = "key"' }
@{ Line = 'Remove-Item Env:\LICENSE_KEY' }
@{ Line = '<Endpoint>${env:FOD_CLOUD_API_URL}</Endpoint>' }
@{ Line = ' url: ${{ env.FOD_CLOUD_API_URL }}' }
@{ Line = '${FOD_CLOUD_API_URL}' }
@{ Line = 'Constants.LEGACY_ENV_VARS' }
@{ Line = ' LICENSE_KEY: ${{ secrets.DEVICE_DETECTION_KEY }}' }
@{ Line = '- RESOURCE_KEY names the resource key' }
@{ Line = 'RESOURCE_KEY=my-key mvn test' }
) {
Get-Kind $Line | Should -Contain 'fallback'
}

# The cloud request engine's tests name the query string parameter
# resource_key, which is on the wire and not in the environment, so the
# old names keep their case.
It 'leaves <Line> alone' -ForEach @(
@{ Line = 'assertEquals("resource_key=" + key, query);' }
@{ Line = ' DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }}' }
@{ Line = ' PAID_RESOURCE_KEY: ${{ secrets.X }}' }
@{ Line = 'System.getenv("_51DEGREES_RESOURCE_KEY")' }
@{ Line = 'System.getenv("_51DEGREES_CLOUD_ENDPOINT")' }
@{ Line = '$env:_51DEGREES_DD_PATH = "file.hash"' }
@{ Line = '$env:_51degrees_dd_path = "file.hash"' }
@{ Line = '${env:_51DEGREES_LICENSE_KEY} = $Keys.TestResourceKey' }
@{ Line = 'assertEquals("${env:user.home}", substitutor.replace(line));' }
@{ Line = ' PAID_RESOURCE_KEY: ${{ env.PAID_RESOURCE_KEY }}' }
@{ Line = 'System.getenv("SUPER_RESOURCE_KEY")' }
@{ Line = 'Set the key with !!YOUR_RESOURCE_KEY!! in the readme.' }
) {
Get-Kind $Line | Should -BeNullOrEmpty
}
}

Describe 'the check over a repository' {

# git quotes a path holding a non-ASCII character unless told not to,
# and a quoted path is not found on disk.
It 'reads a file whose name is not ASCII' {
$repo = Join-Path ([IO.Path]::GetTempPath()) ([Guid]::NewGuid())
New-Item -ItemType Directory $repo | Out-Null
try {
git -C $repo init -q
$name = 'Donn' + [char]0xE9 + 'es.java'
Set-Content -LiteralPath (Join-Path $repo $name) `
-Value 'System.getenv("51DEGREES_DD_PATH");'
git -C $repo add -A
{ & $script:Check -RepoRoot $repo } |
Should -Throw '1 names start with a digit*'
} finally {
Remove-Item -Recurse -Force $repo
}
}
}
124 changes: 124 additions & 0 deletions ci/check-environment-variable-names.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# Every 51Degrees environment variable name in this repository starts with an
# underscore, because a POSIX shell cannot set a name that starts with a digit,
# and each variable has one name with nothing read in its place. This reads
# every file git tracks, which leaves out build output and the submodules, and
# fails on any name that starts with the digit and on any old name that is
# still read.
#
# Dot-sourcing the script defines Find-EnvironmentNameProblem without reading
# the repository, which is how its tests use it.

[CmdletBinding()]
param(
# The root of the checkout to read. The default is the repository this
# script lives in.
[string]$RepoRoot = (Join-Path $PSScriptRoot '..')
)

Set-StrictMode -Version Latest
$ErrorActionPreference = 'Stop'

# The start of a name a shell cannot set, split so this file does not match
# itself.
$digitInitial = '5' + '1DEGREES_'

# A name is not matched when a name character comes before it, so a name with
# a leading underscore is left alone. It is matched in any case, because
# Windows reads an environment variable name in any case, so a lower case
# name that starts with the digit is still one a shell cannot set.
$script:DigitInitialPattern = [regex]::new(
"(?<![A-Za-z0-9_])$digitInitial[A-Za-z0-9_]*",
[Text.RegularExpressions.RegexOptions]::IgnoreCase)

# An old name read or set as an environment variable, or held in a constant to
# fall back to. A Java read takes the name as a string, so a quoted bare name
# is matched, along with the PowerShell and config forms the CI scripts and the
# web example use, and a name at the start of a line past any spaces and a
# list dash, which is how a workflow declares one as a key in an env: block
# and how a readme lists one.
#
# The env token is matched on its own, in any case and with a following
# backslash allowed, because PowerShell braces a name as well as prefixing
# it, reads one through the Env: drive, and because the pipeline config
# lookup and a workflow expression both brace one too. Wanting the dollar
# and the token adjacent misses every braced form.
#
# These keep their case. The cloud request engine's tests name the query string
# parameter resource_key, which differs from RESOURCE_KEY only by case, and a
# query string parameter is on the wire rather than in the environment, so
# matching in any case would fail on a name that is not an environment
# variable at all.
$script:FallbackPattern = [regex]::new(
'(?:["'']|(?i:env)[:.]\\?|\$\{|getenv\(\s*["'']|^[\t\- ]*)' +
'(?:RESOURCE_KEY|LICENSE_KEY|FOD_CLOUD_API_URL)(?![A-Za-z0-9_])|' +
'\bLEGACY_[A-Z_]*ENV_VARS?\b')

# The problems on one line, each as its kind, 'digit' or 'fallback', and the
# text matched.
function Find-EnvironmentNameProblem([string]$Line) {
foreach ($match in $script:DigitInitialPattern.Matches($Line)) {
[pscustomobject]@{ Kind = 'digit'; Value = $match.Value }
}
foreach ($match in $script:FallbackPattern.Matches($Line)) {
[pscustomobject]@{ Kind = 'fallback'; Value = $match.Value }
}
}

if ($MyInvocation.InvocationName -eq '.') {
return
}

$root = (Resolve-Path $RepoRoot).Path
# Paths are listed as they are, so a path holding a non-ASCII character is not
# quoted and is still found.
$tracked = & git -C $root -c core.quotePath=false ls-files
if ($LASTEXITCODE -ne 0) {
throw "git ls-files failed in $root"
}

# The tests of this script name every form it fails on, on purpose.
$ownTests = 'ci/check-environment-variable-names.Tests.ps1'

$found = @()
$fallbacks = @()
$read = 0
$skipped = @()
foreach ($relative in $tracked) {
if ($relative -eq $ownTests) {
continue
}
$path = Join-Path $root $relative
# A submodule is tracked as a directory, and its files belong to another
# repository.
if (-not (Test-Path -LiteralPath $path -PathType Leaf)) {
$skipped += $relative
continue
}
$read++
$lineNumber = 0
foreach ($line in [IO.File]::ReadLines($path)) {
$lineNumber++
foreach ($problem in Find-EnvironmentNameProblem $line) {
$text = "${relative}:${lineNumber}: $($problem.Value)"
if ($problem.Kind -eq 'digit') {
$found += $text
} else {
$fallbacks += $text
}
}
}
}

Write-Output ("$read paths read, $($skipped.Count) skipped as not files: " +
"$($skipped -join ', '), and $ownTests left out")

if ($found.Count -gt 0 -or $fallbacks.Count -gt 0) {
Write-Output $found
Write-Output $fallbacks
throw ("$($found.Count) names start with a digit, which a POSIX shell " +
'cannot set, so each must start with an underscore, and ' +
"$($fallbacks.Count) old names are read, set or kept, where each " +
'variable has one name.')
}

Write-Output 'No tracked file names a variable starting with a digit or an old name.'
2 changes: 1 addition & 1 deletion ci/run-integration-tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ if (-not $TestResourceKey) {
Module = 'web/getting-started.onprem'
Port = 8098
ExampleEnv = @{
'51DEGREES_DD_PATH' = (Resolve-Path "$RepoName/cxx/data/device-detection/TAC-HashV41.hash").Path
'_51DEGREES_DD_PATH' = (Resolve-Path "$RepoName/cxx/data/device-detection/TAC-HashV41.hash").Path
}
}
)
Expand Down
2 changes: 1 addition & 1 deletion examples/device-detection/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ case the example runs until the process is stopped:
PORT=8100 java -jar ./web/getting-started.onprem/target/device-detection-java-examples.web.getting-started.onprem-4.4.20-jar-with-dependencies.jar
```

The on-premise web example takes its data file from the `51DEGREES_DD_PATH`
The on-premise web example takes its data file from the `_51DEGREES_DD_PATH`
environment variable, then from `TestDataFile`, and otherwise uses the free Lite file
in `cxx/data/device-detection`, which it downloads before the server starts if it is
missing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
* <p>
* The configuration file for the pipeline is at src/main/webapp/WEB-INF/51Degrees-OnPrem.xml
* <p>
* The data file is taken from the 51DEGREES_DD_PATH environment variable, then
* The data file is taken from the _51DEGREES_DD_PATH environment variable, then
* from TestDataFile (an environment variable or system property), and otherwise
* the free Lite file in cxx/data/device-detection is used, which is downloaded
* before the server starts if it is missing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
<AutoUpdate>false</AutoUpdate>
<CreateTempDataCopy>false</CreateTempDataCopy>
<!--
The data file is taken from the 51DEGREES_DD_PATH environment
The data file is taken from the _51DEGREES_DD_PATH environment
variable, then from TestDataFile (an environment variable or
system property), and otherwise the free Lite file is used.
-->
<DataFile>${51DEGREES_DD_PATH:-${TestDataFile:-cxx/data/device-detection/51Degrees-LiteV4.1.hash}}</DataFile>
<DataFile>${_51DEGREES_DD_PATH:-${TestDataFile:-cxx/data/device-detection/51Degrees-LiteV4.1.hash}}</DataFile>
<PerformanceProfile>LowMemory</PerformanceProfile>
</BuildParameters>
<BuilderName>DeviceDetectionHashEngine</BuilderName>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@

/**
* Checks the order in which the on-premise web example picks its data file,
* being 51DEGREES_DD_PATH first, then TestDataFile, then the Lite file.
* being _51DEGREES_DD_PATH first, then TestDataFile, then the Lite file.
* System properties stand in for the environment variables, because the
* lookup reads either and a test cannot set an environment variable.
*/
public class OnPremDataFileOptionTest {
private static final String DD_PATH = "51DEGREES_DD_PATH";
private static final String DD_PATH = "_51DEGREES_DD_PATH";
private static final String TEST_DATA_FILE = "TestDataFile";
private static final String LITE =
"cxx/data/device-detection/51Degrees-LiteV4.1.hash";
Expand Down
6 changes: 3 additions & 3 deletions examples/ip-intelligence/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ gives a link to ask for a copy. The Compare, Metrics and Suspicious examples nee
properties, so with a free data file they say which they need and stop.
[Contact us](https://51degrees.com/contact-us?utm_source=github&utm_medium=readme&utm_campaign=51degrees-java&utm_content=readme.md&utm_term=enterprise-data-file) for a copy of
the Enterprise data file. To use it, place it in `cxx/data/ip-intelligence` and pass its path
as the first argument, or set the `51DEGREES_IPI_PATH` environment variable or system
as the first argument, or set the `_51DEGREES_IPI_PATH` environment variable or system
property to its path.

The Enterprise data file is over 6 GB, and the MaxPerformance profile loads all of it into
Expand Down Expand Up @@ -61,7 +61,7 @@ Compare and Suspicious examples with the Enterprise data file are skipped.

The examples locate the data file in the following order:

1. The `51DEGREES_IPI_PATH` environment variable or system property, which can
1. The `_51DEGREES_IPI_PATH` environment variable or system property, which can
be set to an explicit path to the data file. It is used in place of any
file named without a full path.
2. The path given as the first argument, or the free file the example uses
Expand Down Expand Up @@ -104,7 +104,7 @@ environment variable or system property.
| MetadataCloud | How to access the meta-data that relates to the properties available from the cloud IP intelligence service. |

Cloud examples need a resource key. The key is read from the
`51DEGREES_RESOURCE_KEY` environment variable or system property first. The
`_51DEGREES_RESOURCE_KEY` environment variable or system property first. The
legacy `TestResourceKey` name is still supported and is checked second.

The cloud property tiers changed in May 2026. The examples and this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public static void run(String dataFile, InputStream is, OutputStream os) throws
// We use the low memory profile, which reads the data file
// from disk as it is needed, because this example may be
// given the Enterprise data file, which is over 6 GB, through
// its first argument or 51DEGREES_IPI_PATH. MaxPerformance
// its first argument or _51DEGREES_IPI_PATH. MaxPerformance
// loads the whole data file into memory, which makes each
// lookup faster where there is memory to hold it.
.setPerformanceProfile(Constants.PerformanceProfiles.LowMemory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static String findAvailableDataFile(String... candidates) {
* hold an explicit path to the IP Intelligence data file. This name is
* checked first, before any search of the project space.
*/
public static final String IPI_PATH_ENV_VAR = "51DEGREES_IPI_PATH";
public static final String IPI_PATH_ENV_VAR = "_51DEGREES_IPI_PATH";

public static class DatafileInfo {
FiftyOneDataFile fileInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class KeyHelper {
* hold the resource key. This name is checked first, before the legacy
* {@link #TEST_RESOURCE_KEY} name.
*/
public static final String RESOURCE_KEY_ENV_VAR = "51DEGREES_RESOURCE_KEY";
public static final String RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY";
public static final String TEST_RESOURCE_KEY = "TestResourceKey";
static Logger logger = LoggerFactory.getLogger(KeyHelper.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void requireResourceKey_NoKey_FailsNamingEveryName() {
assertEquals("This test calls the live 51Degrees cloud service, so " +
"it needs a resource key and there is nothing to fall back " +
"on. None of _51DEGREES_RESOURCE_KEY, " +
"_51DEGREES_RESOURCE_KEY_BESPOKE, 51DEGREES_RESOURCE_KEY, " +
"_51DEGREES_RESOURCE_KEY_BESPOKE, _51DEGREES_RESOURCE_KEY, " +
"TestResourceKey is set, as an environment variable or as a " +
"system property, tried in that order. The key has to carry " +
"the ip and device products, because the test asks the " +
Expand Down
Loading
Loading