Skip to content

fix: parse ASCII minus in locale-aware number binding#15726

Merged
jamesfredley merged 2 commits into
7.1.xfrom
fix/negative-number-binding-locale
Jun 17, 2026
Merged

fix: parse ASCII minus in locale-aware number binding#15726
jamesfredley merged 2 commits into
7.1.xfrom
fix/negative-number-binding-locale

Conversation

@jamesfredley

Copy link
Copy Markdown
Contributor

Summary

Fixes #15714 by accepting browser-submitted ASCII hyphen-minus values in locale-aware number binding for locales whose DecimalFormat minus sign is U+2212, such as nb_NO.

The earlier PR #15475 correctly scoped the rendering fix to form field input output, but #15714 shows the remaining failure is on the request binding side. The sample application's proposed direction was right: convert a leading ASCII minus to the active locale minus before calling NumberFormat.parse, instead of changing display tags.

Changes

  • Normalize only a leading ASCII minus in LocaleAwareNumberConverter before parsing.
  • Leave g:formatNumber, g:fieldValue, and form field rendering behavior unchanged.
  • Add Spock coverage for Long, BigDecimal, and BigInteger converters in nb_NO and en_US.

Verification

  • ./gradlew :grails-databinding:test --tests "org.grails.databinding.converters.web.LocaleAwareNumberConverterSpec"
  • ./gradlew :grails-databinding:test
  • Oracle review-gate verdict: GREEN

Accept browser-submitted ASCII hyphen-minus when binding numbers for locales whose DecimalFormat minus sign is U+2212. This keeps display formatting unchanged while allowing Long, BigDecimal, and BigInteger converters to parse negative values for nb_NO.

Fixes #15714

Assisted-by: Hephaestus:gpt-5.5 oracle
Copilot AI review requested due to automatic review settings June 10, 2026 21:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes locale-aware request data binding for negative numbers when browsers submit an ASCII hyphen-minus (-, U+002D) but the active locale’s DecimalFormatSymbols.minusSign is a different character (notably U+2212 for locales like nb_NO). It does so by normalizing only a leading ASCII minus to the locale’s expected minus sign before parsing, and adds targeted Spock coverage.

Changes:

  • Normalize a leading ASCII - to the locale-specific minus sign for DecimalFormat-backed parsers in LocaleAwareNumberConverter before calling NumberFormat.parse.
  • Reuse a single formatter instance per conversion and validate parse consumption against the normalized input.
  • Add Spock tests covering Long, BigDecimal, and BigInteger conversions for nb_NO and en_US.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
grails-databinding/src/main/groovy/org/grails/databinding/converters/web/LocaleAwareNumberConverter.groovy Normalizes leading ASCII minus to locale minus for DecimalFormat to prevent parse failures in locales with non-ASCII minus signs.
grails-databinding/src/test/groovy/org/grails/databinding/converters/web/LocaleAwareNumberConverterSpec.groovy Adds regression coverage for negative number parsing across nb_NO and en_US for Long, BigDecimal, and BigInteger.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jamesfredley jamesfredley self-assigned this Jun 10, 2026
@jamesfredley jamesfredley moved this to In Progress in Apache Grails Jun 10, 2026
@jamesfredley jamesfredley added this to the grails:7.1.2 milestone Jun 10, 2026
@testlens-app

testlens-app Bot commented Jun 11, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: b272702
▶️ Tests: 5878 executed
⚪️ Checks: 37/37 completed


Learn more about TestLens at testlens.app.

Object convert(Object value) {
def trimmedValue = value.toString().trim()
def formatter = numberFormatter
def valueToParse = replaceAsciiMinusWithLocaleMinus(formatter, trimmedValue)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the JVM has built in locale handling and yet we still have to work around it?

@bito-code-review

Copy link
Copy Markdown

The JVM's built-in locale handling is often insufficient for web applications because it relies on the server's default locale, which may not match the user's browser-provided locale. This converter is designed to bridge that gap by explicitly handling locale-aware number parsing, ensuring that data binding respects the specific locale context of the incoming request rather than defaulting to the server's environment.

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with the change, but I don't think I fully understand why we have to handle this locale issue directly

@jamesfredley

Copy link
Copy Markdown
Contributor Author

I'm ok with the change, but I don't think I fully understand why we have to handle this locale issue directly

Short version: this isn't Grails working around itself - it's a hard limitation of java.text.DecimalFormat once the JVM uses CLDR locale data (the default since JDK 9). I dug into the JDK internals to be sure we weren't just doing something dumb on our side, and we aren't.

Root cause

For nb_NO (and nn_NO, sv_SE, fi_FI, ...) CLDR defines the minus sign as U+2212 MINUS SIGN, not U+002D ASCII HYPHEN-MINUS. DecimalFormat builds its negative prefix straight from DecimalFormatSymbols.getMinusSign() and matches that prefix literally during parse(). So an ASCII - is simply not recognized as a sign - the parser stops at index 0 and the bind fails.

Probe on JDK 21 (Corretto 21.0.11):

=== Locale nb_NO ===
  minusSign char = U+2212
  negativePrefix = U+2212
  parse("-1"  [U+002D U+0031]) -> null   consumedIndex=0/2   // ASCII minus: FAILS
  parse("...1" [U+2212 U+0031]) -> -1    consumedIndex=2/2   // locale minus: ok
  format(-1)  -> [U+2212 U+0031]

=== Locale en_US ===
  minusSign char = U+002D
  parse("-1"  [U+002D U+0031]) -> -1     consumedIndex=2/2   // ASCII minus: ok

So NumberFormat.getInstance(new Locale("nb","NO")).parse("-1") returns null. en_US only works because its CLDR minus happens to be ASCII - which is exactly why this stayed hidden until a Norwegian-locale browser posted a form.

Why Java won't "just pass it through"

There is no leniency knob for this. DecimalFormat has no "accept either minus" mode, and it has no idea the string arrived from an HTML form. The request side, meanwhile, is always ASCII: per the WHATWG "valid floating-point number" rule an <input type=number> submits ASCII -, and on a text field the user types the - on their keyboard, which is also ASCII. So the wire is ASCII while the locale parser only accepts U+2212, and nothing in the JDK reconciles that gap for us.

The "just use Java's default" options I ruled out

  • -Djava.locale.providers=COMPAT restores the pre-9 ASCII minus, but it's a global, deprecated JVM flag (COMPAT data is being removed) and a wrong fix for one parse path.
  • Forcing symbols.minusSign = '-' makes ASCII parse but then rejects a genuine U+2212 - strictly less lenient than what we ship now.

That's why #15475 fixed the render side (emit ASCII minus into inputs) and this PR fixes the matching bind side. Normalizing only a leading ASCII - to the active locale's minus before parse() is the minimal change that keeps full locale-aware parsing (grouping/decimal separators) intact while accepting the sign every browser actually sends - and the locale's native minus still parses too, so it's strictly more lenient, not less.

@jamesfredley jamesfredley merged commit b259627 into 7.1.x Jun 17, 2026
38 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Apache Grails Jun 17, 2026
@jamesfredley jamesfredley deleted the fix/negative-number-binding-locale branch June 17, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Negative number bug with gsp and fields and Norwegian locale in browser.

3 participants