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

Improve and/or provide control over ETag/If-None-Match logic in HttpEntityMethodProcessor [SPR-13626] #18204

Closed
spring-issuemaster opened this issue Oct 30, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Oct 30, 2015

Rossen Stoyanchev opened SPR-13626 and commented

#18074 ensured ETag/If-None-Match logic applies only to HTTP GET requests. While this logic is HTTP spec compliant, as explained under #18074 it can still cause issues with existing clients that are sending If-None-Match: * despite not being able to deal with 304 responses in the first place.

The purpose of this ticket is to continue the discussion and find a reasonable solution for that problem.


Affects: 4.2.2

Issue Links:

  • #18074 ETag/If-None-Match logic in HttpEntityMethodProcessor should not affect methods other than HTTP GET

Referenced from: commits 8893663

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 30, 2015

Rossen Stoyanchev commented

The logic in HttpEntityMethodProcessor is HTTP spec compliant. If an ETag is added to the ResponseEntity and the request contains If-None-Match or If-Modified-Since it's a matter of putting 2 + 2 together. Arguably in itself this is not the problem.

This seems more a question of adapting to clients that are not spec-compliant or are poorly written. So before we explore any option to disable the functionality wholesale, something I see as equally undesirable and a swing in the opposite direction (effectively poorly written clients dictate server behavior in this regard), I would like to ensure we've done all we can to protect against such clients without the need to disable the feature. For example see my comment under #18074. Let's continue from there.

Francisco Lozano, I'm also curious to hear what you had been doing currently in your application to deal with these clients. You must have some logic that returns 304 or 200?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 3, 2015

Francisco Lozano commented

Thanks for the consideration and for trying to address the issue, but I cannot justify the effort to discuss specifics if the premises are already laid out like that and I have such fundamental disagreement with the general vision.

I have decided to workaround this issue for now, and evaluate other technologies which are more aligned with my own vision and requirements. Again, thanks for the effort and for your work with Spring Framework.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 3, 2015

Juergen Hoeller commented

Francisco Lozano, I have a hard time seeing where the fundamental disagreement with our general vision is. We've been discussing this at our team call yesterday, evaluating what we can do to leniently handle clients which are not quite HTTP spec compliant in this respect. We understand that this can be a regression for some scenarios, and we're trying to do the best we can to retain our out-of-the-box 4.2 feature set while also preserving compatibility with such not-quite-compliant clients.

From my perspective, this is simply how modern-day infrastructure evolves. If we would insist on strict 100% compatibility with the behavior of previous versions of the frameworks, we'd be severely constrained with respect to fine-tuning the out-of-the-box experience for new applications. We're aiming for 99.9% and we're usually quite good at finding a balance eventually, even if it takes a few maintenance releases to get there. In contrast to many other projects (be it open source or commercial), we're willing to have a timely and extended discussion, allowing for different opinions that we're then trying to find a balance for, and we're willing to release a fix for the problem pretty much immediately once consensus is reached (and not half a year or even years later).

All in all, I understand your initial frustration - but I have a hard time understanding the outright rejection of a further discussion when we already made the decision to leniently handle scenarios like yours. It's your call, of course, and your time and effort that needs to be justified and managed. In any case, we are going to proceed with our efforts here and I hope that Spring Framework 4.2.3 will at least be closer to the runtime behavior that you were expecting there.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 6, 2015

Francisco Lozano commented

Juergen Hoeller Thanks for your comment. I don't want to rehash the discussion, but let me explain where the fundamental disagreement I think comes from:

  • I have been using MVC long-time and I have been doing eTag handling myself - custom, maybe correct/incorrect from client handling perspective or even from my own server semantics perspective, but it is exposed as a contract, it is working, and I have compatibility commitments that I need to honour.

  • I update to 4.2 and the behaviour changes, in some ways in backward-incompatible way (debatable), because Spring MVC added support for conditional requests.

  • I cannot disable the above in a decent way, I have to swallow the MVC-provided conditional request support even if it doesn't suit my clients or my server logic, and the Spring MVC team thinks it's OK whereas I think it's not <- that's the disagreement. If the MVC framework approach is that it's OK to add new features that can collide and make opt-out very complicated, I am not so keen on using that framework anymore for my long-term REST API contract commitments. You can improve etag handling for bad clients and that will for sure improve the framework, but the fundamental issue of assuming it's OK to add new features in a 4.X release that may break stuff without the ability to disable them easily is a deal-breaker for me.

That's why I decided I am better off looking for 1. a workaround and 2. carefully look for other, more permanent solution that allows me to avoid Spring MVC for my REST layer so that I can keep this kind of thing under my control, at least in this project.

All of the above doesn't preclude the respect and appreciation I have for your work in all the spring projects I've been using over the years.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 6, 2015

Brian Clozel commented

Francisco Lozano, without taking too much of your time, could give us some real examples of those client requests? HTTP headers should be all we need. This way, we could determine if we can consider this as "incorrect HTTP client behavior" and disable conditional request handling by default in those cases.

We're releasing this version on Monday, so this is our last chance to fix this in a way that suits you.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 6, 2015

Rossen Stoyanchev commented

I cannot disable the above in a decent way, I have to swallow the MVC-provided conditional request support even if it doesn't suit my clients or my server logic, and the Spring MVC team thinks it's OK

It seems you will accept nothing short of a decent way to disable the feature and that's preventing you from seeing how far out of our way we are going to get your feedback. Please understand the perspective of a framework which has to consider not just your application but the implication of decisions on many others. We have to try to put request in perspective and think them through. We can't just act. If we do so we'd be reminded very quickly how we didn't do our homework.

We are very open to giving you what you want but we need to go through the process. There are good reasons why we are asking these question. Even a decent way to disable the feature requires some work and hence motivation. So please work with us and and answer Brian's questions above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.