joerghoh commented on PR #72: URL: https://github.com/apache/sling-org-apache-sling-xss/pull/72#issuecomment-6504966974
I compared the existing ESAPI-backed implementation with this PR and confirmed a compatibility change: ESAPI canonicalizes encoded numeric input before parsing, while the replacement passes it directly to the Java numeric parsers. These three inputs all decode to the decimal text `100`: | Input | Encoding | Decoding | | --- | --- | --- | | `%31%30%30` | URL percent-encoding | `%31` → `1`; each `%30` → `0` | | `100` | HTML decimal character reference | `1` → `1`; the remaining `00` is literal | | `\x31\x30\x30` | JavaScript hexadecimal escapes, supplied as literal backslash sequences | `\x31` → `1`; each `\x30` → `0` | For all three inputs, the existing `getValidInteger`, `getValidLong`, `getValidDouble`, and `getValidDimension` methods return the parsed value (`100`, or `100.0` for double). With this PR they return the caller-provided default instead. This is not a demonstrated injection issue, but it can change the output of callers supplying encoded numeric values. Could we either preserve canonicalization or explicitly document this as an intentional compatibility change and add regression tests defining the expected behavior? The added boundary tests do not cover this difference. Separately, `getValidDouble` now accepts inputs previously rejected by ESAPI: `" 100.5 "` returns `100.5`, `"0x1.0p3"` returns `8.0`, and `"1d"`/`"1f"` return `1.0`. These are safe numeric results, but the migration is not behavior-equivalent; tests should characterize the intended accepted syntax. Validation on PR head `b93c7f6`: `mvn clean verify` passed all 560 tests, including RAT and OSGi baseline checks; `mvn spotless:check` also passed. The packaged bundle retains `org.owasp.shim` and contains no ESAPI classes or configuration. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
