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

All suffix globs except first fail to match if path has . character in prefix section #8184

Closed
markslater opened this issue Jun 21, 2022 · 6 comments · Fixed by #8185
Closed

Comments

@markslater
Copy link
Contributor

Jetty version(s)
11.0.10

Java version/vendor (use: java -version)
openjdk version "11.0.15" 2022-04-19
OpenJDK Runtime Environment (build 11.0.15+10-Ubuntu-0ubuntu0.20.04.1)
OpenJDK 64-Bit Server VM (build 11.0.15+10-Ubuntu-0ubuntu0.20.04.1, mixed mode, sharing)

OS type/version
Linux 5.4.0-120-generic

Description
In Jetty 11.0.9, the path spec *.foo matches all paths that end .foo, such as bar.foo and bar.baz.foo. In Jetty 11.0.10, if more than one suffix glob path spec is defined, that path spec won't match bar.baz.foo because org.eclipse.jetty.http.pathmap.PathMappings attempts to match suffix globs from the first instance of . in the path, and then terminates early.

How to reproduce?
Given the following class:

package com.example;

import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;

import java.io.IOException;
import java.io.Writer;

public class SuffixServer {
    public static void main(String[] args) throws Exception {
        final Server server = new Server(8080);
        final ServletContextHandler servletContextHandler = new ServletContextHandler();
        servletContextHandler.addServlet(new ServletHolder(new FooServlet()), "*.bar");
        servletContextHandler.addServlet(new ServletHolder(new FooServlet()), "*.foo");
        server.setHandler(servletContextHandler);
        server.start();
    }

    private static final class FooServlet extends HttpServlet {
        @Override
        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
            try (Writer responseWriter = resp.getWriter()) {
                responseWriter.write("Foo");
            }
        }
    }
}

Using Jetty 11.0.9, http://localhost:8080/bar.baz.foo returns a 200, but using Jetty 11.0.10, it returns a 404 (both handle http://localhost:8080/bar.foo successfully, as expected).

Note that the line servletContextHandler.addServlet(new ServletHolder(new FooServlet()), "*.bar"); is critical to reproducing the failure because the (alphabetically?) first suffix glob ends up being rechecked on PathMappings#243

Also note that it's possible that 11.0.10 is more correct than 11.0.9. I couldn't find in the Servlet Spec whether *.foo is expected to match bar.baz.foo or not, but intuitively I'd expect it would.

@markslater markslater added the Bug For general bugs on Jetty side label Jun 21, 2022
@joakime joakime added this to To do in Jetty 10.0.11/11.0.11 - 🧊 FROZEN 🥶 via automation Jun 21, 2022
@joakime joakime self-assigned this Jun 21, 2022
@joakime
Copy link
Contributor

joakime commented Jun 21, 2022

This is an ugly bug.
It seems to be related to the optimized group skip.

joakime added a commit that referenced this issue Jun 21, 2022
…pattern

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Jun 21, 2022
…pattern

Cherry-pick of commit d1ecdf6

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor

joakime commented Jun 21, 2022

@joakime
Copy link
Contributor

joakime commented Jun 21, 2022

We'll be spinning a new release once these are green.

joakime added a commit that referenced this issue Jun 21, 2022
…pattern (#8186)

Cherry-pick of commit d1ecdf6

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Jetty 10.0.11/11.0.11 - 🧊 FROZEN 🥶 automation moved this from To do to Done Jun 21, 2022
joakime added a commit that referenced this issue Jun 21, 2022
…pattern (#8185)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime changed the title All suffix globs except first fail to match if path has . character in prefix in Jetty 11.0.10 All suffix globs except first fail to match if path has . character in prefix section Jun 21, 2022
@joakime joakime added this to To do in Jetty 9.4.48 - 🧊 FROZEN 🥶 via automation Jun 21, 2022
@joakime joakime moved this from To do to Done in Jetty 9.4.48 - 🧊 FROZEN 🥶 Jun 21, 2022
@joakime
Copy link
Contributor

joakime commented Jul 5, 2022

@markslater Jetty 9.4.48.v20220622 has this fix (btw)

@markslater
Copy link
Contributor Author

Using it successfully in 11.0.11 :) Thanks for the fast turnaround!

@joakime
Copy link
Contributor

joakime commented Aug 1, 2022

@markslater looks like another suffix bug snuck through in Jetty 9.4.48 as well. #8396

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 High Priority
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants