Skip to content

Fix UrlPathHelper#shouldRemoveSemicolonContent()#27303

Merged
sbrannen merged 1 commit into
spring-projects:mainfrom
evpaassen:fix-27256
Aug 22, 2021
Merged

Fix UrlPathHelper#shouldRemoveSemicolonContent()#27303
sbrannen merged 1 commit into
spring-projects:mainfrom
evpaassen:fix-27256

Conversation

@evpaassen

@evpaassen evpaassen commented Aug 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #27256

This PR fixes a bug that renders read-only UrlPathHelpers unusable, because shouldRemoveSemicolonContent() throws an exception. For example, when using UrlPathHelper#rawPathInstance, which is read-only.

The checkReadOnly() method should only be called from methods that modify properties to prevent modification of read-only instances.

@evpaassen evpaassen mentioned this pull request Aug 20, 2021
Closed
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 20, 2021
@sbrannen

sbrannen commented Aug 20, 2021

Copy link
Copy Markdown
Member

Hi @evpaassen,

Can you please introduce a unit test that fails prior to the change and passes after the change?

@sbrannen sbrannen self-assigned this Aug 20, 2021
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 20, 2021
@sbrannen sbrannen added this to the 5.3.10 milestone Aug 20, 2021
@evpaassen

evpaassen commented Aug 20, 2021

Copy link
Copy Markdown
Contributor Author

Hi @sbrannen,

I added a test now. I'm not sure it really has added value, because nothing regarding the readOnly property in this class was tested before, and the new test is effectively testing a getter method. But on the other hand, it will prevent a regression, I guess.

Please let me know if you're happy with it, or that I should make any changes.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 20, 2021
@evpaassen
Fix UrlPathHelper#shouldRemoveSemicolonContent()
51a1731 
The checkReadOnly() method should only be called from
methods that modify properties to prevent modification
of read-only instances.

Fixes spring-projects#27256
@sbrannen

sbrannen commented Aug 22, 2021

Copy link
Copy Markdown
Member

I added a test now. I'm not sure it really has added value, because nothing regarding the readOnly property in this class was tested before, and the new test is effectively testing a getter method. But on the other hand, it will prevent a regression, I guess.

Yes, my intent was to make sure the problem is fixed and does not return (i.e., to serve as a regression test).

Please let me know if you're happy with it, or that I should make any changes.

That looks good now.

Thanks

@sbrannen sbrannen merged commit 462e19d into spring-projects:main Aug 22, 2021
@evpaassen evpaassen deleted the fix-27256 branch August 22, 2021 12:20
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x labels Aug 22, 2021
@spring-projects-issues spring-projects-issues mentioned this pull request Aug 22, 2021
Closed
@sbrannen sbrannen removed the status: feedback-provided Feedback has been provided label Aug 22, 2021
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Aug 22, 2021
@sbrannen
Polish contribution
99970a5 
See spring-projectsgh-27303
@evpaassen

evpaassen commented Aug 22, 2021

Copy link
Copy Markdown
Contributor Author

You're welcome. And thanks for reviewing and merging!

sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Aug 22, 2021
@evpaassen @sbrannen
Fix UrlPathHelper#shouldRemoveSemicolonContent() (spring-projects#27303)
10a90d3 
The checkReadOnly() method should only be called from
methods that modify properties to prevent modification
of read-only instances.

Fixes spring-projects#27256
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Aug 22, 2021
@sbrannen
Polish contribution
97a89d6 
See spring-projectsgh-27303
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Aug 22, 2021
@sbrannen
Fix Checkstyle violation
b28f403 
See spring-projectsgh-27303
@sbrannen

sbrannen commented Aug 22, 2021

Copy link
Copy Markdown
Member

Note that this was also backported to 5.2.17 in 10a90d3.

@sync-by-unito sync-by-unito Bot mentioned this pull request Sep 15, 2021
Closed
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@evpaassen @lxbzmy
Fix UrlPathHelper#shouldRemoveSemicolonContent() (spring-projects#27303)
de4fdb6 
The checkReadOnly() method should only be called from
methods that modify properties to prevent modification
of read-only instances.

Fixes spring-projects#27256
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@sbrannen @lxbzmy
Polish contribution
3f0caf2 
See spring-projectsgh-27303
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@sbrannen @lxbzmy
Fix Checkstyle violation
0003b1b 
See spring-projectsgh-27303
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@sbrannen sbrannen

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug

Projects

None yet

Milestone

5.3.10

Development

Successfully merging this pull request may close these issues.

UrlPathHelper: checkReadOnly() called in read method shouldRemoveSemicolonContent()

3 participants

@evpaassen @sbrannen @spring-projects-issues