You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "HoustonPutman (via GitHub)" <gi...@apache.org> on 2025/01/31 22:51:09 UTC

[PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

HoustonPutman opened a new pull request, #3154:
URL: https://github.com/apache/solr/pull/3154

   https://issues.apache.org/jira/browse/SOLR-17379


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3154:
URL: https://github.com/apache/solr/pull/3154#issuecomment-2634894329

   > LGTM. Bit confused why BasicAuthIntegrationTest ever passed, but I'm sure you fix did it!
   
   It worked because it just logged an error, it didn't fail the test. Pretty weird, but oh well.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3154:
URL: https://github.com/apache/solr/pull/3154#issuecomment-2632228099

   This fails every time on Crave, because a grade test runner is returning an exit code of "1". This doesn't replicate for me, but it is suspicious given that `TestSecurityManager` does stuff around exit codes... Trying my best to replicate locally


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3154:
URL: https://github.com/apache/solr/pull/3154#discussion_r1941784747


##########
solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java:
##########
@@ -295,12 +295,12 @@ public void testBasicAuth() throws Exception {
       verifySecurityStatus(cl, baseUrl + "/admin/info/key", "key", NOT_NULL_PREDICATE, 20);
       assertAuthMetricsMinimums(17, 8, 8, 1, 0, 0);
 
-      String[] toolArgs = new String[] {"status", "--solr-url", baseUrl};
+      String[] toolArgs =
+          new String[] {"status", "--solr-url", baseUrl, "--credentials", "harry:HarryIsUberCool"};
       ByteArrayOutputStream baos = new ByteArrayOutputStream();
       PrintStream stdoutSim = new PrintStream(baos, true, StandardCharsets.UTF_8.name());
       StatusTool tool = new StatusTool(stdoutSim);
       try {
-        System.setProperty("basicauth", "harry:HarryIsUberCool");

Review Comment:
   I have no idea, clearly it didn't work haha.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3154:
URL: https://github.com/apache/solr/pull/3154#issuecomment-2632284025

   Nvm, replicated it locally.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on code in PR #3154:
URL: https://github.com/apache/solr/pull/3154#discussion_r1940367902


##########
solr/core/src/test/org/apache/solr/update/processor/ParsingFieldUpdateProcessorsTest.java:
##########
@@ -1064,43 +1065,37 @@ public void testAKSTZone() throws IOException {
     final String inputString = "Thu Nov 13 04:35:51 AKST 2008"; // asctime + timezone1
 
     final long expectTs = 1226583351000L;
-    assertEquals(
-        expectTs,
-        DateTimeFormatter.ofPattern(dateFormat, Locale.ENGLISH)
-            .withZone(ZoneId.of("UTC"))
-            .parse(inputString, Instant::from)
-            .toEpochMilli());
 
-    // ensure english locale and root locale return the same date
-    assertEquals(
-        expectTs + "",
-        DateTimeFormatter.ofPattern(dateFormat, Locale.ENGLISH)
-            .withZone(ZoneId.of("UTC"))
-            .parse(inputString, Instant::from)
-            .toEpochMilli(),
-        DateTimeFormatter.ofPattern(dateFormat, Locale.ROOT)
-            .withZone(ZoneId.of("UTC"))
-            .parse(inputString, Instant::from)
-            .toEpochMilli());
+    try {
+      // ensure english locale and root locale return the same date
+      assertEquals(
+          expectTs,
+          DateTimeFormatter.ofPattern(dateFormat, Locale.ENGLISH)
+              .withZone(ZoneId.of("UTC"))
+              .parse(inputString, Instant::from)
+              .toEpochMilli());
+
+      assertEquals(
+          expectTs,
+          DateTimeFormatter.ofPattern(dateFormat, Locale.ROOT)
+              .withZone(ZoneId.of("UTC"))
+              .parse(inputString, Instant::from)
+              .toEpochMilli());
+    } catch (DateTimeParseException e) {
+      // If the JVM's java.locale.providers can't parse these, there is no hope of this test passing
+      assumeNoException("JVM's Locale provider is incompatible with this test", e);
+    }
 
     assertParsedDate(
         inputString,
         Date.from(Instant.ofEpochMilli(expectTs)),
         "parse-date-patterns-default-config");
 
-    // A bug in Java 9 (not in 8) causes this to fail!  (not fixed yet?!)

Review Comment:
   nice clean up



##########
solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java:
##########
@@ -295,12 +295,12 @@ public void testBasicAuth() throws Exception {
       verifySecurityStatus(cl, baseUrl + "/admin/info/key", "key", NOT_NULL_PREDICATE, 20);
       assertAuthMetricsMinimums(17, 8, 8, 1, 0, 0);
 
-      String[] toolArgs = new String[] {"status", "--solr-url", baseUrl};
+      String[] toolArgs =
+          new String[] {"status", "--solr-url", baseUrl, "--credentials", "harry:HarryIsUberCool"};
       ByteArrayOutputStream baos = new ByteArrayOutputStream();
       PrintStream stdoutSim = new PrintStream(baos, true, StandardCharsets.UTF_8.name());
       StatusTool tool = new StatusTool(stdoutSim);
       try {
-        System.setProperty("basicauth", "harry:HarryIsUberCool");

Review Comment:
   any chance that this pattern shows up anywhere else and or is suggested in docs?   Magic system variable?



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3154:
URL: https://github.com/apache/solr/pull/3154#issuecomment-2632468777

   Aha! Tracked it down to `BasicAuthIntegrationTest`. Wow these tests...


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17379: Fix date parsing in Java 23, remove Lucene TestSecurityManager [solr]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman merged PR #3154:
URL: https://github.com/apache/solr/pull/3154


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org