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

Gateway-Mvc FormFilter erase all parameters causing retrive parameter return null in later filter or servlet #3244

Closed
sunhao1256 opened this issue Jan 28, 2024 · 9 comments
Milestone

Comments

@sunhao1256
Copy link
Contributor

Describe the bug
Gateway-Mvc FormFilter erase all parameters causing retrive parameter return null in later filter or servlet

Sample
i have a servlet for a form to post submit , but i can not retrive parameter by using request.getParameter("fool"),after i debug, i found formfilter will remove all parameters , and only offer query params to next filter or servlet

why ? , that will cause the later filter or servlet can not access any parameters.

static HttpServletRequest getRequestWithBodyFromRequestParameters(HttpServletRequest request) throws IOException {
		ByteArrayOutputStream bos = new ByteArrayOutputStream(1024);
		Writer writer = new OutputStreamWriter(bos, FORM_CHARSET);

		Map<String, String[]> form = request.getParameterMap();
		String queryString = request.getQueryString();
		StringBuffer requestURL = request.getRequestURL();
		if (StringUtils.hasText(queryString)) {
			requestURL.append('?').append(queryString);
		}
		UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(requestURL.toString());
//only pass queryParams for later parameters
		MultiValueMap<String, String> queryParams = uriComponentsBuilder.build().getQueryParams();
		for (Iterator<Map.Entry<String, String[]>> entryIterator = form.entrySet().iterator(); entryIterator
				.hasNext();) {
			Map.Entry<String, String[]> entry = entryIterator.next();
			String name = entry.getKey();
			List<String> values = Arrays.asList(entry.getValue());
			for (Iterator<String> valueIterator = values.iterator(); valueIterator.hasNext();) {
				String value = valueIterator.next();
				List<String> queryValues = queryParams.get(name);
				boolean isQueryParam = queryParams.containsKey(name) && queryValues != null
						&& queryValues.contains(value);
				if (!isQueryParam) {
					writer.write(URLEncoder.encode(name, FORM_CHARSET));
					if (value != null) {
						writer.write('=');
						writer.write(URLEncoder.encode(value, FORM_CHARSET));
						if (valueIterator.hasNext()) {
							writer.write('&');
						}
					}
				}
			}
			if (entryIterator.hasNext()) {
				writer.append('&');
			}
		}
		writer.flush();

		ByteArrayServletInputStream servletInputStream = new ByteArrayServletInputStream(
				new ByteArrayInputStream(bos.toByteArray()));
//why only use queryParams 
		return new FormContentRequestWrapper(request, queryParams) {
@sunhao1256
Copy link
Contributor Author

i that we should combine the form map and queryParams to pass to RequestWrapper , i can make a PR

@Interessierter
Copy link

oh thank god I am not alone, I am hitting exactly the same issue: after incorporating spring-cloud-starter-gateway-mvc into an existing mvc app all form post mappings (e.g. @PostMapping(value = "/mapping", params = {"save"})) stopped working because none of the queryparams (that are given naturally as request body using form post) survive this filter.

I did not dive deeply into cloud-gateway-mvc, in my case I do path-based routing and the paths that are handled locally and the others that are proxies are completely disjunct but I guess the flexible nature of this library requires cloud-gateway-filtering of everything?
Is there maybe a workaround to exclude endpoints from filtering until this issue is resolved?

Thanks in advance!

@Interessierter
Copy link

if anyone else hits this before it is fixed in here, this is my workaround (it disables the filter):

@Bean
public FormFilter formFilter() {
    // see https://github.com/spring-cloud/spring-cloud-gateway/issues/3244 - this is currently a bug in spring-cloud-starter-gateway-mvc
    return new FormFilter() {
        @Override
        public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
            chain.doFilter(request, response);
        }
    };
}

I stared at the code and read the comments to no avail - I do not get it, why is this Filter required? Everything is already neatly parsed as parameterMap in the innermost request or is this just the case in my simple setup and more exotic usages (huge payloads?) require this?

@spencergibb
Copy link
Member

This is because of the way servlet containers combine parameters. If this didn't happen, there would be duplicate request parameters.

@spencergibb
Copy link
Member

This test fails without the filter

@Test
void formUrlencodedWorks() {
LinkedMultiValueMap<String, String> formData = new LinkedMultiValueMap<>();
formData.add("foo", "bar");
formData.add("baz", "bam");
// @formatter:off
restClient.post().uri("/post?foo=fooquery").header("test", "formurlencoded").contentType(FORM_URL_ENCODED_CONTENT_TYPE)
.bodyValue(formData)
.exchange()
.expectStatus().isOk()
.expectBody(Map.class).consumeWith(result -> {
Map map = result.getResponseBody();
Map<String, Object> form = getMap(map, "form");
assertThat(form).containsEntry("foo", "bar");
assertThat(form).containsEntry("baz", "bam");
});
// @formatter:on
}

@spencergibb spencergibb added this to the 4.1.2 milestone Mar 8, 2024
@spencergibb
Copy link
Member

Unfortunately, #3249 duplicates the query parameters

@spencergibb spencergibb reopened this Mar 8, 2024
@spencergibb
Copy link
Member

I'm thinking this is a documentation issue. If the custom filter is ordered before FormFilter, you have access to request parameters before filterChain.doFilter(request, response);. All request parameters are available after the filter chain, regardless of order.

@spencergibb
Copy link
Member

I've added documentation as well as properties for disabling filters in #3310

spencergibb added a commit that referenced this issue Mar 19, 2024
This allows them to be used in external configuration.

Fixes gh-3244
@sunhao1256
Copy link
Contributor Author

I've added documentation as well as properties for disabling filters in #3310

Great. RemoveContentLengthFilter also Cause bad influence in some scenarios #3308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants