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

Problem with parsing of form parameters without values in Jetty 12? #11230

Closed
gormpaulsen opened this issue Jan 8, 2024 · 5 comments · Fixed by #11255
Closed

Problem with parsing of form parameters without values in Jetty 12? #11230

gormpaulsen opened this issue Jan 8, 2024 · 5 comments · Fixed by #11255
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@gormpaulsen
Copy link

Jetty version(s)
12.0.5

Jetty Environment
ee10

Java version/vendor (use: java -version)

openjdk version "21.0.1" 2023-10-17 LTS
OpenJDK Runtime Environment Temurin-21.0.1+12 (build 21.0.1+12-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.1+12 (build 21.0.1+12-LTS, mixed mode)

OS type/version
macOS Sonoma 14.2.1

Description

POST-ing a form which contains parameters without values causes the request.getParameterNames() to return wrong values for parameter names.

How to reproduce?

I have this simple servlet running with jetty-ee10-maven-plugin
and Jetty 12.0.5:

package test;

import java.io.IOException;
import java.io.PrintWriter;
import java.util.Enumeration;
import jakarta.servlet.ServletException;
import jakarta.servlet.annotation.WebServlet;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

@WebServlet(value="/", name="helloWorldServlet")
public class HelloWorldServlet extends HttpServlet {

    @Override
    protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        resp.setStatus(200);
        resp.setHeader("Content-Type", "text/plain;charset=utf-8");
        
        try (PrintWriter writer = resp.getWriter()) {
            writer.write("Method: " + req.getMethod() + "\n");
            writer.write("Parameters:\n");
            Enumeration<String> names = req.getParameterNames();
            while (names.hasMoreElements()) {
                String name = names.nextElement();
                writer.write("   " + name + " = '" + req.getParameter(name) + "'\n");
            }
        }
    }
}

When I try this with curl, I get the following output:

> curl -X POST 'http://localhost:8080/?param0=value0&param1' -d 'param2&param3=value3&param4=value4&param5'
Method: POST
Parameters:
   param0 = 'value0'
   param1 = ''
   param2&param3 = 'value3'
   param4 = 'value4'

The parameters supplied in the query string (param0, param1) seem
OK, however there is something strange with the parsing of the form
parameters in the request body (param2&param3 should be separated,
param5 is missing).

This is the output when running the same curl command with the same servlet (only modified to use javax.servlet) running under Jetty 9:

Method: POST
Parameters:
   param5 = ''
   param0 = 'value0'
   param3 = 'value3'
   param4 = 'value4'
   param1 = ''
   param2 = ''
@gormpaulsen gormpaulsen added the Bug For general bugs on Jetty side label Jan 8, 2024
@sbordet
Copy link
Contributor

sbordet commented Jan 8, 2024

Your query string is pretty wrong, as query parameter values must be encoded.

We'll fix the parsing error, but likely we should reject such bad query strings entirely.

@sbordet sbordet self-assigned this Jan 8, 2024
@sbordet
Copy link
Contributor

sbordet commented Jan 8, 2024

Doh, completely misread the single quotes in your example.
I'll try to reproduce.

@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.6 FROZEN Jan 8, 2024
@joakime
Copy link
Contributor

joakime commented Jan 8, 2024

Ah, the old application/x-www-form-urlencoded parsing of blank values issue.

Did you know that the behavior of how to parse blank values is not specified in any spec around application/x-www-form-urlencoded (ietf, w3c, servlet, etc)?

In fact, many languages and libs default to excluding blank values.
You often have to do extra work to get the blank values to be seen.

sbordet added a commit that referenced this issue Jan 8, 2024
Fixed parsing of form parameters in FormFields.parse().
Added more tests for valid edge cases, and invalid cases.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor

sbordet commented Jan 8, 2024

@joakime the parser was broken, it's more than just blank values.

PR #11255 should fix this issue.

sbordet added a commit that referenced this issue Jan 12, 2024
…11255)

Fixed parsing of form parameters in FormFields.parse().
Added more tests for valid edge cases, and invalid cases.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.6 FROZEN Jan 12, 2024
@gregw
Copy link
Contributor

gregw commented Jan 12, 2024

@sbordet does this need to be back ported to 10 and 11?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants