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

ETag header is removed from PUT/POST/PATCH responses [SPR-14767] #19333

Closed
spring-projects-issues opened this issue Sep 29, 2016 · 16 comments
Closed
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 29, 2016

Robert Winkler opened SPR-14767 and commented

Hi,

it seems after this commit (cc5300c) ETag headers are only added to GET/HEAD requests/responses.
We can't add ETag headers to PUT/POST/PATCH responses anymore.

We are doing conditional POST/PUT/PATCH requests with "If-Match" HTTP header to check that the resource has not been modified concurrently by another client. The response either contains a HTTP 200 Code and the ETag header of the updated resource or a HTTP 412 Precondition failed, if the resource has been modified by another client (when the If-Match header does not match the current ETag header of the resource).

Right now we can't return the ETag of the updated resource anymore.

Kind regards,

Robert Winkler


Affects: 4.3.3, 5.0 M2

Reference URL: #18168

Issue Links:

  • #19389 Regression: ETag and Last-Modified headers are removed in non-safe methods that return 200 OK ("is duplicated by")

Referenced from: commits ee17f56, c98cdd4

1 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 29, 2016

Brian Clozel commented

How are you returning that ETag in your controller? Are you returning a ResponseEntity? Are you using ServletWebRequest?
Could you share some code sample that demonstrates how you are achieving this? Maybe we could improve our support to help that use case.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 29, 2016

Robert Winkler commented

Yes sure.
We are returning the ETag from a Controller.

This is how we extract the If-Match header from the request:

@RequestHeader(value = HttpHeaders.IF_MATCH, required = false) String eTag

Then we generate the ETag of the current resource, compare it with the If-Match header and
generate a new ETag from the updated resource, if the If-Match header matches the current ETag.
Otherwise we return a 412 Precondition Failed.

    if (StringUtils.isNotBlank(eTag)) {
            currentETag = // Algorithm to generate the current ETag of the resource
    }

    if (ETagUtils.isETagNotChanged(eTag, currentETag)) {
            // Update resource
            String newETag =  // Algorithm to generate the ETag of the updated resource
            return ResponseEntity.ok()
                    .eTag(newETag)
                    .cacheControl(CacheControlUtils.cacheMustRevalidate())
                    .body(updatedContactResponse);
   } else {
            return new ResponseEntity<>(HttpStatus.PRECONDITION_FAILED);
   }
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 3, 2016

Francisco Lozano commented

We are hitting this condition also. Any other header works, but etag and last-modified don't. Also doing a conditional PUT, with our own handling of If-Match / If-None-Match.

@RequestMapping(value = "/some/path/{objectID}", method = PUT, produces = UpdateResponse.JSON_MIME_TYPE)
@ResponseBody
public ResponseEntity<UpdateResponse> put(
          @RequestHeader(value = IF_MATCH, required = false) final String ifMatch,
          @RequestHeader(value = IF_NONE_MATCH, required = false) final String ifNoneMatch,
          @RequestHeader(value = CONTENT_TYPE, required = false) DataType dataType,
          @PathVariable final String objectID, final InputStream inputStream) {

...

			MyData data = myService.doStuff(objectID, inputStream); // fake
			UpdateResponse response = new UpdateResponse();
			response.setSomeData(data.getSomeData() );

			HttpHeaders responseHeaders = new HttpHeaders();
			responseHeaders.setLastModified(data.getModifiedAt().getTime());
			responseHeaders.setETag(data.getETag().toQuotedString());


			return new ResponseEntity<UpdateResponse>(response, responseHeaders, HttpStatus.OK);
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Francisco Lozano commented

Note that, as of https://tools.ietf.org/html/rfc7231#section-4.3.4, this should be supported:

An origin server MUST NOT send a validator header field
(Section 7.2), such as an ETag or Last-Modified field, in a
successful response to PUT unless the request's representation data
was saved without any transformation applied to the body (i.e., the
resource's new representation data is identical to the representation
data received in the PUT request) and the validator field value
reflects the new representation. This requirement allows a user
agent to know when the representation body it has in memory remains
current as a result of the PUT, thus not in need of being retrieved
again from the origin server, and that the new validator(s) received
in the response can be used for future conditional requests in order
to prevent accidental overwrites (Section 5.2).

The redaction is not very clear (MUST NOT ... unless) but it says that, in case there's no server-side transformation, the ETag / Last-Modified can be returned.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Robert Winkler commented

Since Spring Boot 1.4.1 comes with Spring Framework 4.3.3:
Do you know if Spring Boot 1.4.1 works with Spring Framework 4.3.2 properly as a workaround?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Stéphane Nicoll commented

Robert, could you please avoid cross-posting your questions? This is a waste of time for everybody. Thanks.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Brian Clozel commented

This is now fixed and should be available in the latest 4.3.4.BUILD-SNAPSHOT.
The goal of #18168 was to align our support of conditional requests in both 5.x and 4.3.x branches, but also "spring-webmvc" and "spring-web-reactive" modules. This allows us to improve both modules and backport fixes in a more secure way. Obviously, I missed something here - sorry for the inconvenience.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Robert Winkler commented

Thx Brian.
I assume you mean 4.3.4.BUILD-SNAPSHOT

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Brian Clozel commented

Yes, 4.3.4!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2016

Francisco Lozano commented

Thanks Brian

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 6, 2016

Robert Winkler commented

Hi,

the SNAPSHOT version seems to work.

Thanks

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2016

Robert Winkler commented

Hi,

do you already have a release date for 4.3.4?

Kind regards,
Robert Winkler

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2016

Juergen Hoeller commented

For the latest target dates: https://jira.spring.io/browse/SPR/?selectedTab=com.atlassian.jira.jira-projects-plugin:roadmap-panel

We pulled 4.3.4 forward to Nov 3 in the meantime, with a 4.3.5 to follow ahead of Christmas still.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2016

Robert Winkler commented

Ok. Thx.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 9, 2016

Francisco Lozano commented

One question related to this issue. In this code:

	private boolean isResourceNotModified(ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) {
		ServletWebRequest servletWebRequest =
				new ServletWebRequest(inputMessage.getServletRequest(), outputMessage.getServletResponse());
		HttpHeaders responseHeaders = outputMessage.getHeaders();
		String etag = responseHeaders.getETag();
		long lastModifiedTimestamp = responseHeaders.getLastModified();
		if (inputMessage.getMethod() == HttpMethod.GET || inputMessage.getMethod() == HttpMethod.HEAD) {
			responseHeaders.remove(HttpHeaders.ETAG);
			responseHeaders.remove(HttpHeaders.LAST_MODIFIED);
		}

		return servletWebRequest.checkNotModified(etag, lastModifiedTimestamp);
	}

what is the purpose of removing the ETAG and the LAST_MODIFIED that are provided in the HttpEntity returned by the controller?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 9, 2016

Francisco Lozano commented

Ok, 4.3.4 is now in similar situation to #18204, which I stopped pursuing. I have, again, to stop using MVC in another project if I want to manage bad clients behaviour. This is great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants