Conversation
9b06b9e to
1a66044
Compare
| newPart = newAuthority; | ||
| } else if (oldPart.startsWith("//")) { | ||
| int index = oldPart.indexOf('/', 2); | ||
| int indexSlash = oldPart.indexOf('/', 2); |
There was a problem hiding this comment.
we have to take care of the case where the "/" comes from the query string (because of the call to getSchemeSpecificPart()).
| // Existing fragment | ||
| if (fragment != null) { | ||
| this.internalRef = this.internalRef.substring(0, this.fragmentIndex + 1) + fragment; | ||
| setInternalRef(this.internalRef.substring(0, this.fragmentIndex + 1) + fragment); |
There was a problem hiding this comment.
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.
| // Path found | ||
| final int index2 = oldPart.indexOf('?'); | ||
| final int indexSlash = oldPart.indexOf('/', 2); | ||
| final int indexQuery = oldPart.indexOf('?', 2); |
There was a problem hiding this comment.
we have to take care of the case where the "/" comes from the query string (because of the call to getSchemeSpecificPart()).
| 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; |
There was a problem hiding this comment.
The class has been reworked to make it shorter using Junit parameterized tests
| "http://localhost/abc?query#fragment,/def", | ||
| "http://localhost/abc#fragment?query,/def", | ||
| "http://localhost#fragment/abc?query,/def", | ||
| "http://localhost?query/abc,/def" |
There was a problem hiding this comment.
this case was failing (case where '/' is inside the query)
| Reference originalRef = ReferenceUtils.getOriginalRef(ref, headers); | ||
| assertEquals(originalRef.getSchemeProtocol(), Protocol.HTTPS); | ||
| assertEquals(originalRef.getHostPort(), 123); | ||
| assertEquals(Protocol.HTTPS, originalRef.getSchemeProtocol()); |
There was a problem hiding this comment.
switch the expected value in first position.
| assertEquals("/foo/", parentRef.toString()); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
this part has especially been reworked to make it shorter
| "http://host.com/dir/sub", "http://host.com/dir/sub", | ||
| "http://host.com/dir/sub", null, null); | ||
| @Nested | ||
| class TestParsing { |
There was a problem hiding this comment.
isolate "parsing" test cases (nice presentation in Intellij)
| "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/", |
There was a problem hiding this comment.
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/]
| index = part.indexOf('?'); | ||
| if (index != -1) { | ||
| return part.substring(2, index); | ||
| // At this point, no fragment, but a query string may come from |
There was a problem hiding this comment.
we have to take care of the case where the '/' is inside the query
b45080f to
b8f3258
Compare
| "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," |
There was a problem hiding this comment.
This case was failing ('/' inside query)
| assertEquals("http://localhost:1234/test/last", ref.toString()); | ||
| } | ||
|
|
||
|
|
| ref = new Reference(uri); | ||
| assertEquals("file:///C%7C/wherever%5Cwhatever.swf", ref.toString()); | ||
| } | ||
|
|
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.logging.Level; | ||
| import java.util.regex.Pattern; |
There was a problem hiding this comment.
formatting implies order of "imports" as well... we will take care of that later
b8f3258 to
8928a03
Compare
5b4923e to
ba873a2
Compare
|


The aim
Fix parsing of Reference instances as reported in ticket 1485
Check-list
DO NOT REVIEW