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

NullPointerException with default servlet, include and welcome pages #9966

Closed
grgrzybek opened this issue Jun 26, 2023 · 3 comments · Fixed by #9970 or #9971
Closed

NullPointerException with default servlet, include and welcome pages #9966

grgrzybek opened this issue Jun 26, 2023 · 3 comments · Fixed by #9970 or #9971
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@grgrzybek
Copy link
Contributor

Jetty version(s)
12.0.0 SNAPSHOT

Description
How to reproduce?

I added a comment under #9927, but here's separate issue.

@gregw I can't track the exact part of 0b1c28a, but I think it started causing NPE in some special case.

My servlet is invoked with /gateway/x?what=include&where=/:

String what = req.getParameter("what");
String where = req.getParameter("where");
switch (what) {
	case "redirect":
		resp.sendRedirect(where);
		return;
	case "forward":
		// we can't send anything when forwarding
		req.getRequestDispatcher(where).forward(req, resp);
		return;
	case "include":
		resp.getWriter().print(">>>");
		req.getRequestDispatcher(where).include(req, resp);
		resp.getWriter().print("<<<");
		return;
	default:
}

org.eclipse.jetty.ee10.servlet.ServletApiResponse#getWriter() calls:

String encoding = getServletResponseInfo().getCharacterEncoding(true);

org.eclipse.jetty.ee10.servlet.ServletContextResponse#getCharacterEncoding(boolean) calls:

if (setContentType)
    setCharacterEncoding(encoding, EncodingFrom.DEFAULT);

so org.eclipse.jetty.ee10.servlet.ServletContextResponse#_characterEncoding is set to iso-8859-1.
Because the forward is for /, default servlet is involved (because I need to test welcome files).

org.eclipse.jetty.ee10.servlet.DefaultServlet#doGet() has this fragment:

if (contextResponse != null)
{
    String characterEncoding = contextResponse.getRawCharacterEncoding();
    if (characterEncoding != null)
        content = new ForcedCharacterEncodingHttpContent(content, characterEncoding);
}

and the constructor is:

public ForcedCharacterEncodingHttpContent(HttpContent content, String characterEncoding)
{
    super(content);
    this.characterEncoding = characterEncoding;
    String mimeType = content.getContentTypeValue();
    int idx = mimeType.indexOf(";charset");
    if (idx >= 0)
        mimeType = mimeType.substring(0, idx);
    this.contentType = mimeType + ";charset=" + this.characterEncoding;
}

the content is:

content = {org.ops4j.pax.web.service.jetty.internal.web.DefaultServlet$UnknownLengthHttpContent@4857} "UnknownLengthHttpContent@c898d8f[CompressedFormatsHttpContent@1c61db46[ResourceHttpContent@471a9af8{r=file:/data/sources/github.com/ops4j/org.ops4j.pax.web-jakarta-new/pax-web-jetty/target/b1/,ct=null}]]"
 _delegate: org.eclipse.jetty.http.content.HttpContent  = {org.eclipse.jetty.http.content.PreCompressedHttpContentFactory$CompressedFormatsHttpContent@4919} "CompressedFormatsHttpContent@1c61db46[ResourceHttpContent@471a9af8{r=file:/data/sources/github.com/ops4j/org.ops4j.pax.web-jakarta-new/pax-web-jetty/target/b1/,ct=null}]"
  _delegate: org.eclipse.jetty.http.content.HttpContent  = {org.eclipse.jetty.http.content.ResourceHttpContent@4922} "ResourceHttpContent@471a9af8{r=file:/data/sources/github.com/ops4j/org.ops4j.pax.web-jakarta-new/pax-web-jetty/target/b1/,ct=null}"
   _contentType: java.lang.String  = null
   _etag: org.eclipse.jetty.http.HttpField  = {org.eclipse.jetty.http.PreEncodedHttpField@4925} "ETag: W/"///+d0tLiXs/////73HTsQ""
   _path: java.nio.file.Path  = {sun.nio.fs.UnixPath@4924} "/data/sources/github.com/ops4j/org.ops4j.pax.web-jakarta-new/pax-web-jetty/target/b1"
   _resource: org.eclipse.jetty.util.resource.Resource  = {org.eclipse.jetty.util.resource.PathResource@4749} "file:/data/sources/github.com/ops4j/org.ops4j.pax.web-jakarta-new/pax-web-jetty/target/b1/"
  compressedFormats: java.util.Set  = {java.util.HashSet@4921}  size = 0

the underlying resource is directory, so the extension is * and the resulting content type is null.

so I'm getting NullPointerException.

My test you can run (without bothering with OSGi stuff) is https://github.com/ops4j/org.ops4j.pax.web/blob/1b26a2f8ea394d696a6780ff0fa92b2613019dff/pax-web-jetty/src/test/java/org/ops4j/pax/web/service/jetty/internal/UnifiedJettyTest.java#L361

@grgrzybek grgrzybek added the Bug For general bugs on Jetty side label Jun 26, 2023
@grgrzybek
Copy link
Contributor Author

I think that the default servlet should do:

if (contextResponse != null)
{
    String characterEncoding = contextResponse.getRawCharacterEncoding();
    if (characterEncoding != null)
        content = new ForcedCharacterEncodingHttpContent(content, characterEncoding);
}

only after ensuring that the content is not a directory (or do some other verification).

@lorban lorban self-assigned this Jun 26, 2023
@gregw gregw linked a pull request Jun 26, 2023 that will close this issue
@lorban lorban moved this to 🏗 In progress in Jetty 12.0.0.beta3 Jun 26, 2023
lorban added a commit that referenced this issue Jun 26, 2023
#9966 adapt character encoding when including a path that is a directory listing

Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban moved this from 🏗 In progress to ✅ Done in Jetty 12.0.0.beta3 Jun 26, 2023
@lorban lorban moved this from ✅ Done to 👀 In review in Jetty 12.0.0.beta3 Jun 26, 2023
@lorban
Copy link
Contributor

lorban commented Jun 26, 2023

@grgrzybek I've pushed a fix (see #9970) to the jetty-12.0.x branch. If you've got any feedback, I'd be happy to hear it. Thanks!

@grgrzybek
Copy link
Contributor Author

Works perfectly - thanks @lorban !

@lorban lorban moved this from 👀 In review to ✅ Done in Jetty 12.0.0.beta3 Jun 27, 2023
@lorban lorban closed this as completed Jun 27, 2023
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
None yet
2 participants