Struts 2
  1. Struts 2
  2. WW-3601

Files starting with "struts" (e.g., "struts.jsp") give 404

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0, 2.1.8, 2.2.1.1
    • Fix Version/s: 2.3.1
    • Component/s: None
    • Environment:

      Operating System:Windows 7
      JDK-version:1.6
      Tomcat-version:7.0.6

    • Flags:
      Important

      Description

      I hope I can explain this bug clear. Forgive me my poor English.
      When I make a file, the filename likes 'struts.jsp', I request this page in my IE, for example 'http://localhost/struts.jsp', it tells me a 404-error.
      Then I changed the name to '123struts.jsp', request again, I got the page.
      But if I named the page 'struts123.jsp', it'll give me the 404-error again.
      I also tried to change the suffix, like 'struts.html', while it's happened again, even though I changed the name like 'struts.txt'.
      This bug has occurred in Struts2.0.0, 2.1.8 and 2.2.1.1.
      Of couse, if I do not use the Struts2, this bug disappeared.
      Thanks.

      1. WW-3601.patch
        0.7 kB
        Steven Benitez

        Activity

        Hide
        Hudson added a comment -

        Integrated in Struts2 #302 (See https://builds.apache.org/hudson/job/Struts2/302/)
        WW-3601 - switches to /struts/ and /static/ patterns instead of /struts and /static to handle static content

        lukaszlenart :
        Files :

        • /struts/struts2/trunk/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
        Show
        Hudson added a comment - Integrated in Struts2 #302 (See https://builds.apache.org/hudson/job/Struts2/302/ ) WW-3601 - switches to /struts/ and /static/ patterns instead of /struts and /static to handle static content lukaszlenart : Files : /struts/struts2/trunk/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
        Hide
        Lukasz Lenart added a comment -

        Ok, I solved it as mentioned, we'll see in the future if it was a good choice

        Show
        Lukasz Lenart added a comment - Ok, I solved it as mentioned, we'll see in the future if it was a good choice
        Hide
        Dave Newton added a comment -

        I vote for changing it, since we're not communicating our intent as well as we could in the code.

        Show
        Dave Newton added a comment - I vote for changing it, since we're not communicating our intent as well as we could in the code.
        Hide
        Steven Benitez added a comment -

        I don't see any downside to patching this whatsoever. The intent is clearly for referencing files such as /static/css/my-style.css or /struts/files/awesome.png.

        Show
        Steven Benitez added a comment - I don't see any downside to patching this whatsoever. The intent is clearly for referencing files such as /static/css/my-style.css or /struts/files/awesome.png.
        Hide
        Philip Luppens added a comment -

        I would vote for making the change - based on the principle of the least surprise.

        Show
        Philip Luppens added a comment - I would vote for making the change - based on the principle of the least surprise.
        Hide
        Johannes Geppert added a comment -

        +1

        Show
        Johannes Geppert added a comment - +1
        Hide
        Maurizio Cucchiara added a comment -

        +1 for keeping everything unchanged

        Show
        Maurizio Cucchiara added a comment - +1 for keeping everything unchanged
        Hide
        Lukasz Lenart added a comment -

        I don't know if is worth changing that, how often you must have struts.jsp or static.jsp page ?

        Show
        Lukasz Lenart added a comment - I don't know if is worth changing that, how often you must have struts.jsp or static.jsp page ?
        Hide
        Steven Benitez added a comment - - edited

        You guessed it – the issue is in DefaultStaticContentLoader:345.

        public boolean canHandle(String resourcePath)

        { return serveStatic && (resourcePath.startsWith("/struts") || resourcePath.startsWith("/static")); }

        I think it is safe to change it to check for "/struts/" or "/static/" without worry that this would break anything. The expectation is that you would be referencing an individual file, for which there would always be a trailing slash after those strings.

        Show
        Steven Benitez added a comment - - edited You guessed it – the issue is in DefaultStaticContentLoader:345. public boolean canHandle(String resourcePath) { return serveStatic && (resourcePath.startsWith("/struts") || resourcePath.startsWith("/static")); } I think it is safe to change it to check for "/struts/" or "/static/" without worry that this would break anything. The expectation is that you would be referencing an individual file, for which there would always be a trailing slash after those strings.
        Hide
        Dave Newton added a comment -

        Subject edit, downgrade.

        Show
        Dave Newton added a comment - Subject edit, downgrade.
        Hide
        Dave Newton added a comment -

        I'd assume the existing S2 code uses "/struts" as the static request path without checking for any other characters, like a trailing "/". This is certainly not a blocking bug, so I'm downgrading it.

        What does everyone else think about fixing this? I don't have a problem with it, but I also haven't tested the change. I'd be a little weirded out if fixing this broke anybody's code, but you never know.

        Show
        Dave Newton added a comment - I'd assume the existing S2 code uses "/struts" as the static request path without checking for any other characters, like a trailing "/". This is certainly not a blocking bug, so I'm downgrading it. What does everyone else think about fixing this? I don't have a problem with it, but I also haven't tested the change. I'd be a little weirded out if fixing this broke anybody's code, but you never know.

          People

          • Assignee:
            Lukasz Lenart
            Reporter:
            Li Hao Su
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development