Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UriComponentsBuilder handles invalid port numbers correctly #26905

Closed
wants to merge 1 commit into from
Closed

UriComponentsBuilder handles invalid port numbers correctly #26905

wants to merge 1 commit into from

Conversation

glqdlt
Copy link
Contributor

@glqdlt glqdlt commented May 7, 2021

Hello.
Fixed a bug in the'UriComponentBuilder' class.
The bug occurs in'UriComponentBuilder#fromUriString()'.
A bug occurs when the port of the argument is not a number.

This issue occurs with the 'PORT_PATTERN' regular expression.
If port is not a number, the regular expression is not captured.
This is a good regex, but it causes problems.

You can check the issue in the test code below.

UriComponents stringPort = UriComponentsBuilder.fromUriString("http://localhost:port/path").build();
assertThat(stringPort.getScheme()).isEqualTo("http");
assertThat(stringPort.getHost()).isEqualTo("localhost");
assertThat(stringPort.getPath()).isEqualTo("/path"); 
// expected: "/path", but was "port/path"

'PORT' that is not captured is captured by'PATH_PATTERN'.
private static final String PATH_PATTERN = "([^?#]*)";

I have fixed this bug.
Thank you.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 7, 2021
@rstoyanchev rstoyanchev self-assigned this May 7, 2021
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 7, 2021
@rstoyanchev rstoyanchev added this to the 5.3.7 milestone May 7, 2021
@glqdlt glqdlt changed the title Fix bug when 'UriComponentsBuilder#fromUriString()' port was not numer. Fix bug when 'UriComponentsBuilder#fromUriString()' port was not number. May 7, 2021
@glqdlt glqdlt force-pushed the fix/UriComponentsBuilderNonNumberPort branch from 4e11755 to fd6659e Compare May 7, 2021 15:34
@glqdlt
Copy link
Contributor Author

glqdlt commented May 7, 2021

Sorry. I made a mistake.
Corrected a typo in the PR title and commit message.

(before) Fix bug when 'UriComponentsBuilder#fromUriString()' port was not numer.
(after) Fix bug when 'UriComponentsBuilder#fromUriString()' port was not number.

rstoyanchev pushed a commit that referenced this pull request May 7, 2021
@glqdlt @rstoyanchev
UriComponentsBuilder handles invalid port correctly
65797d0 
See gh-26905
@rstoyanchev rstoyanchev changed the title Fix bug when 'UriComponentsBuilder#fromUriString()' port was not number. UriComponentsBuilder handles invalid port numbers correctly May 7, 2021
@nguyensach nguyensach mentioned this pull request May 8, 2021
Closed
@glqdlt glqdlt mentioned this pull request May 10, 2021
Closed
@glqdlt
Copy link
Contributor Author

glqdlt commented May 10, 2021

Hello @rstoyanchev
Sorry, there is a serious bug in this pull request.
i fixed a bug (#26918), Could you please review it?
Thank you.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented May 10, 2021 •
edited

This has broken a Boot test case with a URL such as "http://localhost:52567?trace=false&message=false".

@rstoyanchev rstoyanchev reopened this May 10, 2021
@rstoyanchev
Copy link
Contributor

@glqdlt my apologies, I saw your comment but missed the fact you had opened a separate PR. For future reference, simply push your changes to the same branch in order to update the PR.

@glqdlt
Copy link
Contributor Author

glqdlt commented May 11, 2021

@rstoyanchev thank you very much for the comments.

@sync-by-unito sync-by-unito bot mentioned this pull request May 12, 2021
Merged
@izeye izeye mentioned this pull request May 18, 2021
Merged
@bclozel bclozel mentioned this pull request Jun 8, 2021
Closed
bclozel added a commit that referenced this pull request Jun 8, 2021
@bclozel
Revisit fix for gh-26905 in UriComponentsBuilder
e35fe33 
This commit revisits the recently updated `PORT_PATTERN` in
`UriComponentsBuilder`. The fix introduced in gh-26905 fails with
ambiguous URL patterns, especially when the port and path parts of the
pattern are hard to differentiate, for example
"https://localhost:{port}{path}".

This commit reinstates the previous behavior without undoing the actual
fix. The only limitation introduced here is the fact that only a single
pattern variable is allowed for the port pattern part.

Fixes gh-27039
@sync-by-unito sync-by-unito bot mentioned this pull request Jun 10, 2021
Closed
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
@glqdlt @Zoran0104
UriComponentsBuilder handles invalid port correctly
7d69a6c 
See spring-projectsgh-26905
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
@rstoyanchev @Zoran0104
Polishing contribution
a9d0b83 
Closes spring-projectsgh-26905
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
@rstoyanchev @Zoran0104
Follow-up fix for recent change to PORT_PATTERN
641ad38 
The pattern was changed in 65797d0
to check for characters up until "/", instead of for digits, but now
also checks up until "?" and "#".

Closes spring-projectsgh-26905
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this pull request Aug 20, 2021
@bclozel @Zoran0104
Revisit fix for spring-projectsgh-26905 in UriComponentsBuilder
f283866 
This commit revisits the recently updated `PORT_PATTERN` in
`UriComponentsBuilder`. The fix introduced in spring-projectsgh-26905 fails with
ambiguous URL patterns, especially when the port and path parts of the
pattern are hard to differentiate, for example
"https://localhost:{port}{path}".

This commit reinstates the previous behavior without undoing the actual
fix. The only limitation introduced here is the fact that only a single
pattern variable is allowed for the port pattern part.

Fixes spring-projectsgh-27039
@fanjiwang1992 fanjiwang1992 mentioned this pull request Nov 8, 2021
Closed
@happyWilliam0 happyWilliam0 mentioned this pull request Nov 10, 2021
Closed
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@glqdlt @lxbzmy
UriComponentsBuilder handles invalid port correctly
416a023 
See spring-projectsgh-26905
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@rstoyanchev @lxbzmy
Polishing contribution
f52547a 
Closes spring-projectsgh-26905
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@rstoyanchev @lxbzmy
Follow-up fix for recent change to PORT_PATTERN
c6396bd 
The pattern was changed in 65797d0
to check for characters up until "/", instead of for digits, but now
also checks up until "?" and "#".

Closes spring-projectsgh-26905
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@bclozel @lxbzmy
Revisit fix for spring-projectsgh-26905 in UriComponentsBuilder
62a026a 
This commit revisits the recently updated `PORT_PATTERN` in
`UriComponentsBuilder`. The fix introduced in spring-projectsgh-26905 fails with
ambiguous URL patterns, especially when the port and path parts of the
pattern are hard to differentiate, for example
"https://localhost:{port}{path}".

This commit reinstates the previous behavior without undoing the actual
fix. The only limitation introduced here is the fact that only a single
pattern variable is allowed for the port pattern part.

Fixes spring-projectsgh-27039
@sbrannen sbrannen mentioned this pull request May 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@rstoyanchev rstoyanchev

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Milestone
5.3.7
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@glqdlt @rstoyanchev @spring-projects-issues