Skip to content

Fix the use of quickstart with a combined base resource. - #15556

Open
lachlan-roberts wants to merge 3 commits into
jetty-12.1.xfrom
fix/jetty-12.1.x/QuickStart-CombinedResource
Open

Fix the use of quickstart with a combined base resource.#15556
lachlan-roberts wants to merge 3 commits into
jetty-12.1.xfrom
fix/jetty-12.1.x/QuickStart-CombinedResource

Conversation

@lachlan-roberts

Copy link
Copy Markdown
Member

In the case of a CombinedResource as the base resource, context.getWar() will return null, which was causing this ISE to be thrown.

We should instead test the value of context.getBaseResource() due to the fact that CombinedResource cannot be converted to String by context.getWar().

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
lachlan-roberts requested a review from joakime August 6, 2026 05:53
@lachlan-roberts lachlan-roberts self-assigned this Aug 6, 2026
@lachlan-roberts lachlan-roberts added the Bug For general bugs on Jetty side label Aug 6, 2026
@lachlan-roberts lachlan-roberts added the Sponsored This issue affects a user with a commercial support agreement label Aug 6, 2026
@lachlan-roberts lachlan-roberts moved this to 👀 In review in Jetty 12.1.13 Aug 6, 2026

@joakime joakime left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly questions at this point.

//check that webapp is suitable for quick start - it is not a packed war
String war = context.getWar();
if (StringUtil.isBlank(war) || !context.getBaseResource().isDirectory())
Resource baseResource = context.getBaseResource();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is setWar() and setBaseResource() modifying the same variable?
In past versions, this was 2 separate concepts that got selected/merged at this point.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is setWar() and setBaseResource() modifying the same variable?

No, but they are still closely related. For example setBaseResource() may be called with a resource obtained from getWar() by the WebInfConfiguration.

Or if setWar() was not explicitly set, then getWar() will initialize _war with a string obtained from the base resource path.

}

@Test
public void testQuickStartWithCombinedBaseResource() throws Exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an existing testcase that only uses WebAppContext.setWar()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test directly above this (testDuplicateGenerationFromContextXml) only uses setWar and not setBaseResource().

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
if (StringUtil.isBlank(war) || !context.getBaseResource().isDirectory())
Resource baseResource = context.getBaseResource();
if (!Resources.isReadableDirectory(baseResource))
throw new IllegalStateException("Invalid Quickstart location");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add the baseResource in the ISE message to help quickly figuring out what the location is when this problem occurs.

Maybe something like: "Invalid Quickstart location, base resource is not a readable directory: " + baseResource?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have done this now for both EE10 and EE11.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
lachlan-roberts requested a review from lorban August 21, 2026 04:20
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 Sponsored This issue affects a user with a commercial support agreement

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

3 participants