Make StringQuoter date parsing thread-safe

[metsw24-max]

RequestFactory and AutoBean decode Date values on the server through StringQuoter.tryParseDate, which shared two static SimpleDateFormat instances across all request threads. SimpleDateFormat parsing mutates internal Calendar state, so concurrent requests race and return wrong dates or throw. Give each thread its own formatter via ThreadLocal.

[niloc132]

Interesting - the code is definitely wrong, but does this actually fail in any real use case? It looks like you could manufacture such a scenario by manually writing JSON, but clearly AutoBeans (and thus, RequestFactory) won't serialize dates using these formats. I imagine these were speculatively added, but never actually used.

gwt/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java

Lines 113 to 115 in 4e6ae10

	public Splittable encode(Object value) {
	return StringQuoter.create(String.valueOf(((Date) value).getTime()));
	}

That seems contradicted by discussion at #6330 (esp #6330 (comment)) and 3df95e6. Nevertheless, there are still no tests using date strings that I can find.

I'm apprehensive about merging this despite its correctness, given the abandoned #10307 and the behavior of the account (many PRs to many unrelated repos with weak followup and tendency towards LLM-like content).

[vjay82]

If the code was never used, then it could be removed, no? However, if it is in use, it looks more correct than before, proposed by a LLM or not, no?

[niloc132]

Right - the code looks right, but the use case needs tests. The linked issue makes me suspect it is (or at least "was") used in some capacity, so removing a feature of a public api is at least somewhat dangerous, and there's probably no downside... but especially if it is tool-generated code, it should come with a test.

As a maintainer, I think it is appropriate for me to have a degree of skepticism for "more correct than before" code coming from low-effort sources, as a way to gain trust and attack the project. I'm definitely not accusing @metsw24-max here - the fixes to all the various repos they've sent code to seem well-intentioned, but very few have tests or actual use cases.

[metsw24-max]

Fair point on the missing tests. Pushed a JRE test (StringQuoterJreTest) that covers the millis, ISO-8601, Z-suffix, RFC 2822, and unparseable paths, plus a concurrent hammer on tryParseDate that fires 16 threads x 2000 parses at the same date string. On the unpatched code it reliably produces wrong Dates (got ~11 on my box); with the ThreadLocal it sits at zero. Also wired into AutoBeanSuite.

[niloc132]

Thanks - locally I haven't had the test pass without the fix, though I did see only one wrong date value (out of 16*2000 checks) - usually single digit, but rather less than 5, so the test does what it is there to do.

[zbynek]

Why wasn't this caught by https://errorprone.info/bugpattern/DateFormatConstant ? The rule exists in 2.33.