Skip to content

Conversation

@markt-asf
Copy link
Contributor

Implements the solution discussed in #20.

I plan on leaving this open for a week or two for discussion before merging and wouldn't be surprised if changes were required.

@markt-asf
Copy link
Contributor Author

markt-asf commented Jun 25, 2025

Re-based and made a couple clarifications to the specification text. No functional change.

@markt-asf
Copy link
Contributor Author

Some feedback from the Tomcat community:

At the moment, a <welcome-servlet> is almost certain to match since it will likely be using extension mapping making any welcome resources that follow unnecessary. This might be an issue for web fragments since any welcome resources defined in a fragment will be added to the end of the existing list of welcome resources.

Is this a problem? I think probably yes, but would welcome additional views. If it is an issue, then one way to resolve it would be to change the mapping algorithm to:

For each welcome resource in the order it appears in the deployment descriptor:

  • append the partial URI to the request URI to create a welcome URI
  • if the welcome URI has an exact match to a servlet mapping, send the request to that servlet
  • if the welcome URI has a prefix match to a servlet mapping, send the request to that servlet
  • if a static file exists in the web application for the welcome URI, send the request to that static resource

If no match is found then for each legacy welcome file and welcome-servlet in the order they appear in the deployment descriptor:

  • append the partial URI to the request URI to create a welcome URI
  • if a servlet mapping (other than the default servlet) matches the welcome URI, send the request to that servlet

This does mean two passes of the welcome files are more likely but I think it is closer to the current behaviour whilst fixing the problem this change was intended to fix. I'm currently leaning towards this alternative approach.

Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

Sorry, coming at this very late.
I'm not understanding why in the 6.2 descriptor that welcome-file resources can be matched to servlets?

Also, I don't think this well handles the case of '*.jsp', where if a file exists AND there is a servlet mapping, then dispatch to the servlet.

TCK, Javadoc and other updates to follow once the spec language is
agreed.
@markt-asf
Copy link
Contributor Author

This version should:

  • be backwards compatible with the current behaviour
  • solve the original issue (index.jsp masking index.do when index.jsp does not exist)

Pre-compiled JSPs where the original JSP has been removed are handled by the 3rd bullet of the first pass (they will be an exact match to a servlet mapping)

index.jsp masking index.do when index.jsp does not exist is handled by:

  • index.jsp is a welcome file
  • index.do is a welcome servlet
  • first pass skips index.jsp as no matching static file is found
  • first pass skips index.do as it is a welcome-servlet
  • second pass skips index.jsp as is is a welcome-file (this is where things go wrong in <6.2 because it matches and is the issue that this change is intended to address)
  • second pass matches index.do as there is a matching servlet mapping (*.do) that isn't the default servlet

Fragments inserting new welcome files that are masked by index.do is handled by:

  • index.do is a welcome servlet
  • index.flt is a welcome file
  • first pass skips index.do because it is a welcome servlet
  • first pass matches index.flt as the static file exists (<6.2 gets this right too)

I haven't updated the Javadoc, TCK etc. yet. Once we have agreement on the spec language, I'll update everything else to match.

@markt-asf
Copy link
Contributor Author

@gregw Is this any better?

@markt-asf markt-asf changed the base branch from master to main October 15, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants