fix(1485) Reference parsing issues by thboileau · Pull Request #1486 · restlet/restlet-framework-java

thboileau · 2026-02-22T21:40:36Z

The aim

Fix parsing of Reference instances as reported in ticket 1485

Check-list

PR size
- Under 300 lines ✅
- Can't be split without complicating the process even more
Tests
- Added
- Not applicable (why?)
Doc
- Added
- Not applicable
Reviewer
- Asked for a review
- Added label DO NOT REVIEW

thboileau · 2026-02-22T22:02:46Z

org.restlet.java/org.restlet/src/main/java/org/restlet/data/Reference.java

 			newPart = newAuthority;
 		} else if (oldPart.startsWith("//")) {
-			int index = oldPart.indexOf('/', 2);
+			int indexSlash = oldPart.indexOf('/', 2);


we have to take care of the case where the "/" comes from the query string (because of the call to getSchemeSpecificPart()).

thboileau · 2026-02-22T22:05:05Z

org.restlet.java/org.restlet/src/main/java/org/restlet/data/Reference.java

 			// Existing fragment
 			if (fragment != null) {
-				this.internalRef = this.internalRef.substring(0, this.fragmentIndex + 1) + fragment;
+				setInternalRef(this.internalRef.substring(0, this.fragmentIndex + 1) + fragment);


Intellij complains when this.internalRef is set with a value computed thanks to itself, because it's a volatile class member. It leads to use a setter.

thboileau · 2026-02-22T22:05:51Z

org.restlet.java/org.restlet/src/main/java/org/restlet/data/Reference.java

-					// Path found
-					final int index2 = oldPart.indexOf('?');
+				final int indexSlash = oldPart.indexOf('/', 2);
+				final int indexQuery = oldPart.indexOf('?', 2);


we have to take care of the case where the "/" comes from the query string (because of the call to getSchemeSpecificPart()).

thboileau · 2026-02-22T22:07:13Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

-import org.restlet.engine.header.HeaderConstants;
-import org.restlet.engine.util.ReferenceUtils;
-import org.restlet.util.Series;
+import static org.junit.jupiter.api.Assertions.assertEquals;


The class has been reworked to make it shorter using Junit parameterized tests

thboileau · 2026-02-22T22:07:44Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

+             "http://localhost/abc?query#fragment,/def",
+             "http://localhost/abc#fragment?query,/def",
+             "http://localhost#fragment/abc?query,/def",
+             "http://localhost?query/abc,/def"


this case was failing (case where '/' is inside the query)

thboileau · 2026-02-22T22:08:19Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

        Reference originalRef = ReferenceUtils.getOriginalRef(ref, headers);
-        assertEquals(originalRef.getSchemeProtocol(), Protocol.HTTPS);
-        assertEquals(originalRef.getHostPort(), 123);
+        assertEquals(Protocol.HTTPS, originalRef.getSchemeProtocol());


switch the expected value in first position.

thboileau · 2026-02-22T22:08:50Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

        assertEquals("/foo/", parentRef.toString());
    }

-    /**


this part has especially been reworked to make it shorter

thboileau · 2026-02-22T22:09:28Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

-                "http://host.com/dir/sub", "http://host.com/dir/sub",
-                "http://host.com/dir/sub", null, null);
+    @Nested
+    class TestParsing {


isolate "parsing" test cases (nice presentation in Intellij)

thboileau · 2026-02-22T22:12:25Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

                        "http://www.gamasutra.com/view/feature/177267/devil_may_cry_born_again.php"),
                ref).getTargetRef();
        assertEquals(
-                "http://twitter.com?status=RT%20@gamasutra:%20%20Devil%20May%20Cry%20:%20Born%20Again%20http:?status=RT%20@gamasutra:%20%20Devil%20May%20Cry%20:%20Born%20Again%20http://www.gamasutra.com/view/feature/177267/",


The previous test was wrong.
It showed this log

Can't parse hostPort : [hostRef,requestUri]=[null,http://twitter.com?status=RT%20@gamasutra:%20%20Devil%20May%20Cry%20:%20Born%20Again%20http:?status=RT%20@gamasutra:%20%20Devil%20May%20Cry%20:%20Born%20Again%20http://www.gamasutra.com/view/feature/177267/]

thboileau · 2026-02-22T22:13:32Z

org.restlet.java/org.restlet/src/main/java/org/restlet/data/Reference.java

-			index = part.indexOf('?');
-			if (index != -1) {
-				return part.substring(2, index);
+			// At this point, no fragment, but a query string may come from


we have to take care of the case where the '/' is inside the query

thboileau · 2026-02-23T13:52:19Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

+                "http://localhost/?query,http,localhost,/,query,",
+                "http://localhost/#?query,http,localhost,/,,?query",
+                "http://localhost/path#frag/ment,http,localhost,/path,,frag/ment",
+                "http://localhost/path?qu/ery,http,localhost,/path,qu/ery,"


This case was failing ('/' inside query)

thboileau · 2026-02-23T13:53:20Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

        assertEquals("http://localhost:1234/test/last", ref.toString());
    }

+


thboileau · 2026-02-23T13:53:42Z

org.restlet.java/org.restlet/src/test/java/org/restlet/data/ReferenceTestCase.java

        ref = new Reference(uri);
        assertEquals("file:///C%7C/wherever%5Cwhatever.swf", ref.toString());
    }
+


thboileau · 2026-02-23T20:48:18Z

org.restlet.java/org.restlet/src/main/java/org/restlet/data/Reference.java

 import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Level;
+import java.util.regex.Pattern;


formatting implies order of "imports" as well... we will take care of that later

thboileau force-pushed the 1485_reference_parsing_issues branch 2 times, most recently from 9b06b9e to 1a66044 Compare February 22, 2026 21:46

thboileau commented Feb 22, 2026

View reviewed changes

fix(1485) Reference parsing issues

b8f3258

thboileau force-pushed the 1485_reference_parsing_issues branch from b45080f to b8f3258 Compare February 22, 2026 22:19

thboileau requested a review from jlouvel February 22, 2026 22:28

thboileau commented Feb 23, 2026

View reviewed changes

		assertEquals("http://localhost:1234/test/last", ref.toString());
		}

Conversation

thboileau commented Feb 22, 2026

The aim

Check-list

Uh oh!

thboileau Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thboileau Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thboileau Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thboileau Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thboileau Feb 22, 2026 •

edited

Loading

thboileau Feb 22, 2026 •

edited

Loading

thboileau Feb 22, 2026 •

edited

Loading

thboileau Feb 22, 2026 •

edited

Loading