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

HeaderResultMatchers susceptible to slight variations in the date format [SPR-17330] #21864

Closed
spring-projects-issues opened this issue Oct 3, 2018 · 5 comments
Assignees
Labels
in: test in: web status: backported type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

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

cfloersch opened SPR-17330 and commented

So the server now appears to format HTTP date headers using a single or double digit DAY depending on the time of month. However, the HeaderResultMatchers.dateValue() still uses a two digit DAY zero padded if necessary when doing the matching.

This results in test failing 100% when run between the 1st and 9th of any given month.

Both dates should use the same format. Or better yet the matcher should operate on Date objects rather than formatted strings.

Example build failure

[ERROR] Failures:
[ERROR] SimpleControllerTest.testGetCustomHeader:152 Response header 'Last-Modified' expected:<Wed, 03 Oct 2018 17:56:00 GMT> but was:<Wed, 3 Oct 2018 17:56:00 GMT>


Affects: 5.0.9

Attachments:

Referenced from: commits b89cb20, 658c7f9

Backported to: 5.0.10

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 10, 2018

Rossen Stoyanchev commented

Both dates should use the same format. Or better yet the matcher should operate on Date objects rather than formatted strings.

The response header is a string at that point, so either the input long should be formatted too, or the response header needs to be parsed. We can use HttpHeaders to do either one of those and that way it should be consistent.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 12, 2018

Rossen Stoyanchev commented

Sorry, but I'm having trouble reproducing the issue.

I can see that HttpHeaders formats and stores dates as you indicated. However, when ResponseEntity.ok().lastModified(..).body(..) is returned from the controller, that goes through MockHttpServletResponse#setDateHeader which formats with a 2 digit day. That explains why tests like these have not failed.

Can you provide instructions or sample code how to reproduce the issue?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 12, 2018

cfloersch commented

I attached a sample app. It appears that if the Last-Modified header is set using Headers.setDate(String, long) from a ResponseAdvice you can reproduce the issue. Using ResponseEntity does not produce the issue.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 12, 2018

Rossen Stoyanchev commented

Okay so it all depends on whether the header is set on the MockHttpServletResponse as a String or a long in which can result in slightly different formats. As there is nothing wrong with 2 digits for the day, I've opted to improve the checks in HeadersResultMatchers to parse the actual header which can be done more leniently and accept slightly differences like that.

@spring-projects-issues spring-projects-issues added in: test status: backported type: enhancement in: web labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 5.1.1 milestone Jan 11, 2019
@rstoyanchev
Copy link
Contributor

@rstoyanchev rstoyanchev commented Mar 11, 2019

Note that there was a fix in Spring MVC Test #21864 to make it more lenient in this regard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test in: web status: backported type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants