Skip to content

ACC-2584: fix If-Match and If-None-Match validation for GET on entity item and to-one relation endpoints#226

Merged
NielsCW merged 11 commits intomainfrom
ACC-2584
Feb 4, 2026
Merged

ACC-2584: fix If-Match and If-None-Match validation for GET on entity item and to-one relation endpoints#226
NielsCW merged 11 commits intomainfrom
ACC-2584

Conversation

@NielsCW
Copy link
Contributor

@NielsCW NielsCW commented Jan 28, 2026

this breaks behavior of If-None-Match on entity item requests (it was not supported on relation requests). Fixed in this PR as well.

@NielsCW NielsCW requested a review from a team as a code owner January 28, 2026 10:28
@NielsCW NielsCW force-pushed the ACC-2584 branch 3 times, most recently from 291e135 to 6c76f7d Compare January 30, 2026 14:33
@NielsCW NielsCW changed the title ACC-2584: add If-Match validation for entity item and to-one relation requests ACC-2584: fix If-Match and If-None-Match validation for GET on entity item and to-one relation endpoints Jan 30, 2026
var request = (HttpServletRequest) webRequest.getNativeRequest();

List<VersionConstraint> notMatching = List.of();
if (!isSafeMethod(request.getMethod())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure how I feel about conditionally ignoring the If-None-Match header here.

If it's not combined with the explicit check in the body, the requested version check is not performed, and that's not really obvious from the code in the controller.

Did you take a look at the content controller code?

There is first a check done with WebRequest#checkNotModified(); and then a VersionConstraint check after it. Moving that into an utility method that is called from the 3 places might be nicer?

var eTag = calculateETag(content);
// First check not-modified for a 304 response before performing If-Match validation (with a 412 response)
if(webRequest.checkNotModified(eTag)) {
return null; // The response headers inside webRequest are modified as side effect
}
if(!versionConstraint.isSatisfiedBy(content.getVersion())) {
throw new UnsatisfiedVersionException(
content.getVersion(),
versionConstraint
);
}

@NielsCW NielsCW force-pushed the ACC-2584 branch 3 times, most recently from fa10a5e to 6760ac1 Compare February 2, 2026 14:27

/**
* Verifies whether the version is satisfied by the versionConstraint. Returns true
* when the response is 304 Not Modified, false when the response is 200 Ok.
Copy link
Member

@rschev rschev Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this javadoc should be slightly more like the javadoc of webRequest.checkNotModified(). The way it's phrased here makes it sound like the response has already happened.

Checks whether the requested entity's version matches the versionConstraint.
Returns true when it matches, false when the entity is newer. This will also
set the webRequest's HTTP response status to 304 and 200, respectively.

@NielsCW NielsCW requested a review from rschev February 3, 2026 15:14
@NielsCW NielsCW enabled auto-merge February 4, 2026 08:00
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 4, 2026

@NielsCW NielsCW merged commit 9a5873c into main Feb 4, 2026
3 checks passed
@NielsCW NielsCW deleted the ACC-2584 branch February 4, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants