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.

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

Negative byte values not properly converted to unsigned int in URI encoding #24413

Closed
Andy-2639 opened this issue Jan 22, 2020 · 3 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@Andy-2639
Copy link

Affects: 5.2.0-Release, current master (2020-01-22)

Line numbers from 5.2.0.
spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java:330

		for (byte b : bytes) {
			if (b < 0) {
				b += 256;
			}
			if (type.isAllowed(b)) {
				bos.write(b);
			}
		[...]

The b += 256 is a no-op as b is a byte (and so everything is modulo 256).
type.isAllowed(b) expects an int as argument --> a negative b is widened to a negative int value.

Proposed fix: change the data type of b from byte to int.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 22, 2020
@rstoyanchev rstoyanchev self-assigned this Jan 23, 2020
@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 Jan 23, 2020
@rstoyanchev rstoyanchev added this to the 5.2.4 milestone Jan 23, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 23, 2020

Yes this is for non-ASCII characters where the sign bit makes them appear as negative. We should change that to int c = b & 0xff, effectively Byte.toUnsignedInt. Note that the current behavior ends up in the same outcome because isAllowed will return false just the same.

@rstoyanchev rstoyanchev changed the title URI encoding: negative byte values not properly coverted to non-negative int Negative byte values not properly converted to unsigned int in URI encoding Jan 23, 2020
@Andy-2639
Copy link
Author

I didn't know Byte.toUnsignedInt. Why not just using it?

@rstoyanchev
Copy link
Contributor

I opted to remove the code in the end. It was a no-op after all and whether it's negative or over 128, it makes no difference and isAllowed will return false for both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants