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

JSP temp directory regression, possibly due to fix for #12044 #12124

Closed
scantor opened this issue Aug 1, 2024 · 15 comments · Fixed by #12129
Closed

JSP temp directory regression, possibly due to fix for #12044 #12124

scantor opened this issue Aug 1, 2024 · 15 comments · Fixed by #12129
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@scantor
Copy link

scantor commented Aug 1, 2024

The 12.0.12 patch for issue 12044 seems to have possibly caused a regression in the JSP temp folder creation step.

Starting 12.0.12 I get this:
Caused by: java.lang.IllegalStateException: Could not create JSP scratch directory

I can see on 12.0.11 I get a jsp folder under "tmp/jetty-0_0_0_0-80-idp_war-_idp-any-4380240850460342364"

On 12.0.12, it can't create it. I think the internal name of the tmp folder is being munged so by the time it goes to create the jsp folder, it isn't using the right path. I noticed I think an extra hyphen or possibly some other stray character in the folder name it created, so something seems to have gone wrong somewhere, like the folder created on disk doesn't match the internal variable holding the folder path it defaults the jsp path to.

@scantor scantor added the Bug For general bugs on Jetty side label Aug 1, 2024
@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

Are you using Windows?
What JVM?

Please don't delete the issue template next time.

@scantor
Copy link
Author

scantor commented Aug 1, 2024

It's Linux, Coretto 17.

There is some anecdotal evidence it may not affect the Windows version, I don't have it to test fully.

In my case, it was a straight upgrade (stop 0.11, update my init script pointer, start 0.12) resulting in the exception in the log. If nothiing obvious shows up, once I'm done with some other work I can do a bit more digging tomorrow and compare the tmp folders I'm getting between the two versions, I am 99% sure they were subtly different.

@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

Are you manually configuring the temp directory?
Either at the server level, or at the context level, or at the webapp level?

@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

Are you using a classic jetty-home / jetty-base setup?
Or custom jetty-home / jetty-base?
Or embedded?
When this occurs is it happening on a normal command line? or docker?
If using docker, are you using the official jetty.docker images for Coretto 17 and Jetty 12?

@scantor
Copy link
Author

scantor commented Aug 1, 2024

I am configuring the temp directory with an ini file via -Djava.io.tmpdir=tmp (located in my jetty-base), and the top level webapp and nested webinf folder are created without any problems. On 12.0.11, it also creates the nested jsp subfolder, whereas on 12.0.12, that step fails, taking down the webapp.

I'm not sure what constitutes "classic", but I have jetty-home in /opt, just unpacking the distribution as posted, and my jetty-base is elsewhere, and my init script passes in the necessary variables to point it to them.

Nothing embedded, no.

My startup is via an init script that's fairly exotic due to use of setuid (working fine) and not having managed to get a systemd unit file to work so I do some crazy --dry-run stuff that has been working fine with 12.0.11, unchanged from Jetty 11.

Permissions all seem fine, evidenced by the fact that it's able to create all the other material in tmp.

Red Hat 9 is the only place I've specifically reproduced the bug.

No Docker whatsoever.

If not for the fact that my colleague reported Windows as working, I would be certain this is a regression from the cleanup fix. I'm still close to certain, simply from Occam's Razor.

@scantor
Copy link
Author

scantor commented Aug 1, 2024

If it helps, this is what 12.0.11 looks like. I can get a sample of the partial pre-crash content on 12.0.12 tomorrow latest.

[shibboleth@otdi-ssoqa01 ~]$ ls -al jetty/tmp
total 4
drwxrwx---.  6 shibboleth shibboleth 4096 Aug  1 14:49 .
drwxr-x---. 11 shibboleth shibboleth  132 Jul 15 10:52 ..
drwxrwx---.  4 jetty      shibboleth   31 Aug  1 14:49 jetty-0_0_0_0-80-idp_war-_idp-any-7687450141413216206

[shibboleth@otdi-ssoqa01 ~]$ ls -al jetty/tmp/jetty-0_0_0_0-80-idp_war-_idp-any-7687450141413216206/
total 4
drwxrwx---. 4 jetty      shibboleth   31 Aug  1 14:49 .
drwxrwx---. 6 shibboleth shibboleth 4096 Aug  1 14:49 ..
drwxrwx---. 3 jetty      shibboleth   17 Aug  1 14:50 jsp
drwxrwx---. 3 jetty      shibboleth   21 Aug  1 14:49 webinf

@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

so I do some crazy --dry-run stuff that has been working fine with 12.0.11, unchanged from Jetty 11.

Note that there are few more <parts> available on --dry-run=<part>,<part> on Jetty 12 that might be useful to you.
See start.jar --help for details.

@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

The "jsp" subdirectory is the Apache Jasper "scratchdir".

Internally, on startup of the ServletContext, specifically the JspServlet, the scratchdir init-param is looked for, if it doesn't exist, it uses ...

/* ensure scratch dir */
File scratch;
if (getInitParameter("scratchdir") == null)
{
File tmp = (File)getServletHandler().getServletContext().getAttribute(ServletContext.TEMPDIR);
scratch = new File(tmp, "jsp");
setInitParameter("scratchdir", scratch.getAbsolutePath());
}
scratch = new File(getInitParameter("scratchdir"));
if (!scratch.exists() && !scratch.mkdir())
throw new IllegalStateException("Could not create JSP scratch directory");

So if the temp directory doesn't exist, the call File.mkdir() will fail.

@scantor
Copy link
Author

scantor commented Aug 1, 2024

Right. My impression is that the defaulted in scratchdir built from line 685-687 is not in fact the same path as what's been placed on disk previously, so the folder in the middle of the path doesn't exist.

It wasn't obvious where I should define my own scratchdir if that were a workaround, but I don't know that I'd want to go that route anyway. If it would have to be in my actual webapp's servlet declarations, that's "non-ideal" for sure.

@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

the jsp servlet definition, typically seen in etc/webdefault-ee10.xml would be the place to add/change arbitrary init-param stuff.

Just copy the one from ${jetty.home/etc/ into your ${jetty.base}/etc/ and edit the copied one.

<!-- ==================================================================== -->
<!-- JSP Servlet -->
<!-- This is the jasper JSP servlet. -->
<!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
<!-- The JSP page compiler and execution servlet, which is the mechanism -->
<!-- used by the jsp container to support JSP pages. Traditionally, -->
<!-- this servlet is mapped to URL pattern "*.jsp". -->
<!-- See https://eclipse.dev/jetty/documentation/ -->
<!-- for applicable configuration params. -->
<!-- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -->
<servlet id="jsp">
<servlet-name>jsp</servlet-name>
<servlet-class>org.eclipse.jetty.ee10.jsp.JettyJspServlet</servlet-class>
<init-param>
<param-name>xpoweredBy</param-name>
<param-value>false</param-value>
</init-param>
<init-param>
<param-name>compilerTargetVM</param-name>
<param-value>1.8</param-value>
</init-param>
<init-param>
<param-name>compilerSourceVM</param-name>
<param-value>1.8</param-value>
</init-param>
<load-on-startup>0</load-on-startup>
</servlet>

@scantor
Copy link
Author

scantor commented Aug 1, 2024

Ok, I'm in the midst of my main upgrade to 12.0.11 so once I get back to ground zero I can do the comparison and also try some workarounds. I have feelers out to others who can refute or reproduce my results.

@pbhenson
Copy link
Contributor

pbhenson commented Aug 2, 2024

I tested this, and it appears that 12.0.12 creates the jsp temp directory in jetty_base rather than in the specific tmpdir for the webapp in question:

# find /var/lib/jetty/jsp
/var/lib/jetty/jsp
/var/lib/jetty/jsp/org
/var/lib/jetty/jsp/org/apache
/var/lib/jetty/jsp/org/apache/jsp
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF/jsp
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF/jsp/metadata_jsp.java
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF/jsp/metadata_jsp.class
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF/jsp/status_jsp.java
/var/lib/jetty/jsp/org/apache/jsp/WEB_002dINF/jsp/status_jsp.class
/var/lib/jetty/jsp/org/apache/jsp/index_jsp.java
/var/lib/jetty/jsp/org/apache/jsp/index_jsp.class

Scott's jetty service account doesn't have write access to jetty_base, which is presumably why he's getting that error.

Also, it appears that regardless of whether persistTempDirectory is not specified, is set to true, or is set to false, the webapp temporary directory files are always deleted. Although the jsp tmpdir in the wrong location remains...

This is definitely a change in behavior from earlier versions, and I don't think having to go manually change parameters is the right answer to fixing it. Also, while I'm only running one webapp on this server, it seems if multiple jsp webapps were in place they'd be sharing a temporary directory which seems inadvisable and possibly a security issue. If you can confirm this, you might want to pull this version.

Thanks...

@janbartel
Copy link
Contributor

@scantor @pbhenson next time (if there is a next time) could you please include all of the information on the issue template? Most particularly it is critical for us to know which EE environment you are using. As neither of you gave the environment, I've assumed ee10 and tested that first, and can confirm there is no problem. On a hunch, I tested ee9, and it seems that the problem lies there - it would have been better to know that up front.

Investigating now.

janbartel added a commit that referenced this issue Aug 2, 2024
Also ensure the name of war appears in the name of the tmp dir for ee9
webapps.
@janbartel
Copy link
Contributor

OK, found the problem. I've raised #12129 to fix.

As a workaround in the meanwhile, you could try explicitly setting the location of the tmp directory for each webapp (WebAppContext.setTempDirectory(File).

@scantor
Copy link
Author

scantor commented Aug 2, 2024

Thank you. I didn't realize the extent to which the implementation was that distinct across EE versions, that seems likely to be a maintenance fiasco. Never occured to me it might impact the behavior.

janbartel added a commit that referenced this issue Aug 5, 2024
* Issue #12124 fix jsp scratchdir location for ee9.

Also ensure the name of war appears in the name of the tmp dir for ee9
webapps.
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