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

Fix constant value in XContentTypeOptionsServerHttpHeadersWriter #13155

Closed
marcusdacoregio opened this issue May 11, 2023 · 3 comments
Closed
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@marcusdacoregio
Copy link
Contributor

The value in

should be changed to X-Content-Type-Options

@marcusdacoregio marcusdacoregio added in: web An issue in web modules (web, webmvc) type: bug A general bug labels May 11, 2023
@jzheaux jzheaux added the type: breaks-passivity A change that breaks passivity with the previous release label May 12, 2023
@jzheaux
Copy link
Contributor

jzheaux commented May 12, 2023

Because this field is public, perhaps it would be better to add a new value and deprecate this one? If so, I think we can move the breaks-passivity label` to a new ticket that removes the deprecated value.

@marcusdacoregio marcusdacoregio removed the type: breaks-passivity A change that breaks passivity with the previous release label May 12, 2023
@joerg-richter-5234
Copy link
Contributor

If the following meets the results of your discussion, i'll gladly open a pull request for this issue

  • Deprecate constant X_CONTENT_OPTIONS
  • Introduce constant X_CONTENT_TYPE_OPTIONS
  • Update referenced constant in CONTENT_TYPE_HEADERS from X_CONTENT_OPTIONS to X_CONTENT_TYPE_OPTIONS
public class XContentTypeOptionsServerHttpHeadersWriter implements ServerHttpHeadersWriter {

	@Deprecated( since = "5.7", forRemoval = true)
	public static final String X_CONTENT_OPTIONS = "X-Content-Options";

	public static final String X_CONTENT_TYPE_OPTIONS = "X-Content-Type-Options";

	public static final String NOSNIFF = "nosniff";

	/**
	 * The delegate to write all the cache control related headers
	 */
	private static final ServerHttpHeadersWriter CONTENT_TYPE_HEADERS = StaticServerHttpHeadersWriter.builder()
			.header(X_CONTENT_TYPE_OPTIONS, NOSNIFF).build();

	@Override
	public Mono<Void> writeHttpHeaders(ServerWebExchange exchange) {
		return CONTENT_TYPE_HEADERS.writeHttpHeaders(exchange);
	}

}

@jzheaux
Copy link
Contributor

jzheaux commented May 15, 2023

Thanks for the offer, @joerg-richter-5234. The ticket is yours.

I think there are actually two things going on here. @marcusdacoregio is pointing out that the value is incorrect, and I am pointing out that the variable itself is not ideal.

So, instead, let's please just focus for now on correcting the value to X-Content-Type-Options in 5.7 and onward and leave the variable name as the same.

Please make this change on the 5.7.x branch, including a test to make sure the functionality is correctly verified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
Status: Done
Development

No branches or pull requests

3 participants