Jetty 12 - General cleanup of URIUtil #8861
Conversation
joakime
commented
Nov 3, 2022
joakime
commented
- Removal of __CHARSET
- Removal of unused methods
- Using new switch/case concepts
+ Removal of __CHARSET + Removal of unused methods + Using new switch/case concepts
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
Outdated
Show resolved
Hide resolved
+ collapse separate methods + tag methods only used in URIUtilTest
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
Outdated
Show resolved
Hide resolved
+ simplify encodePath()
| // control characters | ||
| ENCODE_PATH_NEEDS_ENCODING[0x7f] = true; | ||
| for (int i = 0; i < 0x20; i++) | ||
| ENCODE_PATH_NEEDS_ENCODING[i] = true; |
There was a problem hiding this comment.
I believe SPACE 0x20 should be encoded too.
There was a problem hiding this comment.
Already covered in the special chars above
There was a problem hiding this comment.
added testcase specific for this to show it's protected
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
| } | ||
|
|
||
| // Only URIUtil is using this method | ||
| // Only used by URIUtil |
There was a problem hiding this comment.
Can't make it private, as it's tested in URIUtilTest.
And protected isn't valid.
There was a problem hiding this comment.
Then the comment is wrong, should be Only used by URIUtil*Test*.
| @@ -1622,6 +1465,7 @@ static boolean equalsIgnoreEncodings(String uriA, String uriB) | |||
| } | |||
| } | |||
|
|
|||
| // Only used by URIUtil | |||
There was a problem hiding this comment.
Can't make it private, as it's tested in URIUtilTest.
And protected isn't valid.
There was a problem hiding this comment.
Then the comment is wrong, should be Only used by URIUtil*Test*.
There was a problem hiding this comment.
It's used by URIUtil as a sub-method, and that sub-method is tested in URIUtilTest, it's not exposed to others.
There was a problem hiding this comment.
Went ahead and cleaned it out
jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java
Outdated
Show resolved
Hide resolved
