Upgrade GeoServer Core Spring to 5.3.23 and Spring Security to 5.7.3
Description
Environment
Attachments
relates to
Activity
Peter Rushforth March 22, 2023 at 10:14 PM
Peter Rushforth March 20, 2023 at 9:38 PM
Apologies for adding this here; maybe the MapML preview issue is not related to this change, but in case it may be, please see below.
I found that the response for text/html and text/css resources was not being closed; that was not the case for text/javascript resources; they were being served fast and the response stream was being closed. So I noticed that in web.xml the GZIPFilter was being mapped to text.* but not to applications/javascript (it was mapped to application/x-javascript, which I don’t see being used). Anyway, by removing the text.* pattern and not gzip encoding the responses, the big delay is gone and the responses are closed promptly, on main / 2.24-SNAPSHOT, and presumably 2.23-RC1. These considerations don’t seem to apply to the openlayers preview equivalent web resources; I didn’t notice them not working correctly, and they were gzipped. So I don’t know what to make of the issue. I chaged the order of the filter mapping for gzip filter (put it second last), but that didn’t seem to make any difference. I am wondering if the filter is not closing the response for some reason that is only revealed by this change. Thank you.
Peter Rushforth March 20, 2023 at 7:49 PM
So there are 4 different RequestMatchers added to a linked list by this block, and I believe the matches method is invoked automagically, and while we see logging regarding the 3/4 types of matchers, we don’t see the 4th, which I presume corresponds to the matcher that gets invoked. Oddly we don’t see this logging in the 2.22.x branch, only in main.
Peter Rushforth March 20, 2023 at 6:25 PM
Also, in the 2.24-SNAPSHOT branch (main), there are log statements of this form:
[filter.GeoServerSecurityMetadataSource] - Did not match request to ...
and each is labelled (1/4), (2/4), (3/4), (19 occurrences of each), but there are no occurrences of (4/4), which maybe corresponds to something not completed, or at least not logging its completion.
I’ll look into filter.GeoServerSecurityMetadataSource now.
Peter Rushforth March 20, 2023 at 5:51 PM
Hi Sorry a bit of trouble when posting to the list, or so I thought.
Here’s some work I’ve done to track down the issue.
I attach a couple of logs with GEOSERVER_DEVELOPER_LOGGING enabled, the first with a (custom) build from the 2.22.x branch. The customization is that I include some geoserver.mapml logging statements. The second is from main today.
They compare pretty closely, except for the big pause. One difference I've noticed that Joe mentioned is the 2.22.x branch's:
"[filter.GeoServerSecurityContextPersistenceFilter$1] - SecurityContextHolder now cleared, as request processing completed"
log statements don't have any parallel in today's 2.24.SNAPSHOT (main) log, so I'm wondering if there is some resource that is being withheld for many / all the resources.
I think that difference corresponds with the replacement of the SecurityContextPersistenceFilter
with the OncePerRequestFilter
as Joe mentioned below. I don’t know if that’s the culprit, but I do see in devTools on the browser that the text/html and .css requests are processed quickly but remain open for the whole 20-30s of the request.
I believe that the problem relates to the fact that at the previous version 2.22.x, for gzip encoded responses, it seems that no Content-length: header was being sent. At the current revision, the Content-length: header is being set to the length of the unencoded data, and I believe it should either not be set at all, or set to the length of the gzip-encoded data. I will try to fix it, please indicate the preferred path: calculate the correct gzipped size, or omit the header altogether.