Tapestry 5
  1. Tapestry 5
  2. TAP5-1973

In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL

    Details

      Description

      An error in the code means that when secure is true and the port is set to 443, then ":443" is appended:

      int port = secure ? secureHostPort : hostPort;
      String portSuffix = "";

      if (port <= 0)

      { port = request.getServerPort(); int schemeDefaultPort = request.isSecure() ? 443 : 80; portSuffix = port == schemeDefaultPort ? "" : ":" + port; }

      else if (secure && port != 443) portSuffix = ":" + port;
      else if (port != 80) portSuffix = ":" + port;

      String hostname = "".equals(this.hostname) ? request.getServerName() : this.hostname.startsWith("$") ? System.getenv(this.hostname.substring(1)) : this.hostname;

      return String.format("%s://%s%s", secure ? "https" : "http", hostname, portSuffix);

      secure == true && port != 443 is false, so
      port != 80 is evaluated, it true, so
      ":443" is appended.

        Issue Links

          Activity

          Howard M. Lewis Ship created issue -
          Howard M. Lewis Ship made changes -
          Field Original Value New Value
          Assignee Howard M. Lewis Ship [ hlship ]
          Howard M. Lewis Ship made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Howard M. Lewis Ship made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Howard M. Lewis Ship made changes -
          Labels fixed-in-5.4-js-rewrite
          Howard M. Lewis Ship made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 5.3.5 [ 12322440 ]
          Resolution Fixed [ 1 ]
          Hide
          Lenny Primak added a comment -

          I run my production Tapestry apps behind Apache and AJP/JK2 protocol.
          In this setup, serverPort is always 8009, and thus always fails the expectedPort test.
          This means the new way of appending :443. :80 ports to URLs is broken in my setup.

          Since https:// always defaults to :443 and http:// to 80, why is there any appending going on at all?
          This should be removed, or at least make it removed by default, and optionally turned on.

          Show
          Lenny Primak added a comment - I run my production Tapestry apps behind Apache and AJP/JK2 protocol. In this setup, serverPort is always 8009, and thus always fails the expectedPort test. This means the new way of appending :443. :80 ports to URLs is broken in my setup. Since https:// always defaults to :443 and http:// to 80, why is there any appending going on at all? This should be removed, or at least make it removed by default, and optionally turned on.
          Howard M. Lewis Ship made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Howard M. Lewis Ship made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Fix Version/s 5.4 [ 12316401 ]
          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #977 (See https://builds.apache.org/job/tapestry-trunk-freestyle/977/)
          TAP5-1973: :443 added to URLs when using the Link.toAbsoluteURI(true) (Revision cf87299fd4b9140aabd5b1e58ddb373fea59a1f4)

          Result = FAILURE
          hlship :
          Files :

          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #977 (See https://builds.apache.org/job/tapestry-trunk-freestyle/977/ ) TAP5-1973 : :443 added to URLs when using the Link.toAbsoluteURI(true) (Revision cf87299fd4b9140aabd5b1e58ddb373fea59a1f4) Result = FAILURE hlship : Files : tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java
          Hide
          Alejandro Scandroli added a comment -

          This fix has a side effect.
          As commented in http://tapestry.1045711.n5.nabble.com/Tapestry-HTTPS-redirect-to-port-80-td5719759.html
          If SymbolConstants.HOSTPORT and SymbolConstants.HOSTPORT_SECURE are not set, then when redirecting from insecure to secure you end up with and URL like this one: https://yourhost:80

          Here is the test to expose the little bug:

          @Test
          public void secure_url_without_condifured_hostports()

          { expect(request.getServerPort()).andReturn(80).once(); expect(request.isSecure()).andReturn(false).once(); replay(); BaseURLSource baseURLSource = new BaseURLSourceImpl(request, "localhost", 0, 0); assertEquals(baseURLSource.getBaseURL(true), "https://localhost"); verify(); }

          A posible solution would be to only use request.getServerPort() if request.isSecure() == secure

          Can this issue be reopened or should I file a new issue with the proposed solution?

          Show
          Alejandro Scandroli added a comment - This fix has a side effect. As commented in http://tapestry.1045711.n5.nabble.com/Tapestry-HTTPS-redirect-to-port-80-td5719759.html If SymbolConstants.HOSTPORT and SymbolConstants.HOSTPORT_SECURE are not set, then when redirecting from insecure to secure you end up with and URL like this one: https://yourhost:80 Here is the test to expose the little bug: @Test public void secure_url_without_condifured_hostports() { expect(request.getServerPort()).andReturn(80).once(); expect(request.isSecure()).andReturn(false).once(); replay(); BaseURLSource baseURLSource = new BaseURLSourceImpl(request, "localhost", 0, 0); assertEquals(baseURLSource.getBaseURL(true), "https://localhost"); verify(); } A posible solution would be to only use request.getServerPort() if request.isSecure() == secure Can this issue be reopened or should I file a new issue with the proposed solution?
          Kalle Korhonen made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Assignee Howard M. Lewis Ship [ hlship ] Kalle Korhonen [ kaosko ]
          Hide
          Howard M. Lewis Ship added a comment -

          I'm not sure why you re-opened this. At least in the 5.4 code it looks good. 5.3 code is identical.

          Show
          Howard M. Lewis Ship added a comment - I'm not sure why you re-opened this. At least in the 5.4 code it looks good. 5.3 code is identical.
          Howard M. Lewis Ship made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Hide
          Lenny Primak added a comment -

          As far as I know the bug still exists, as commented by Alejandro.
          He also has a good test for this
          That's why I reopened it.

          This is exposed when running behind apache / mod_jk combination

          Show
          Lenny Primak added a comment - As far as I know the bug still exists, as commented by Alejandro. He also has a good test for this That's why I reopened it. This is exposed when running behind apache / mod_jk combination
          Hide
          Howard M. Lewis Ship added a comment -

          Ok, I missed his comment. I look into this.

          Show
          Howard M. Lewis Ship added a comment - Ok, I missed his comment. I look into this.
          Howard M. Lewis Ship made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Assignee Kalle Korhonen [ kaosko ] Howard M. Lewis Ship [ hlship ]
          Hide
          Lenny Primak added a comment -

          Also, take a look at my comment from 21/Oct/12 23:26
          When running behind Apache/mod_jk, port is 8009, so that's why
          this bug manifests itself with this setup.

          Show
          Lenny Primak added a comment - Also, take a look at my comment from 21/Oct/12 23:26 When running behind Apache/mod_jk, port is 8009, so that's why this bug manifests itself with this setup.
          Howard M. Lewis Ship made changes -
          Summary :443 added to URLs when using the Link.toAbsoluteURI(true) In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL
          Hide
          ASF subversion and git services added a comment -

          Commit 30a66c9526dee3da55a5204e43a169fe4692df79 in tapestry-5's branch refs/heads/master from Howard M. Lewis Ship
          [ https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;h=30a66c9 ]

          TAP5-1973: In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL

          Show
          ASF subversion and git services added a comment - Commit 30a66c9526dee3da55a5204e43a169fe4692df79 in tapestry-5's branch refs/heads/master from Howard M. Lewis Ship [ https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;h=30a66c9 ] TAP5-1973 : In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL
          Howard M. Lewis Ship made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Fix Version/s 5.3.5 [ 12322440 ]
          Resolution Fixed [ 1 ]
          Howard M. Lewis Ship made changes -
          Link This issue is a clone of TAP5-2366 [ TAP5-2366 ]
          Hide
          Hudson added a comment -

          ABORTED: Integrated in tapestry-trunk-freestyle #1293 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1293/)
          TAP5-1973: In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL (hlship: rev 30a66c9526dee3da55a5204e43a169fe4692df79)

          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java
          Show
          Hudson added a comment - ABORTED: Integrated in tapestry-trunk-freestyle #1293 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1293/ ) TAP5-1973 : In certain development cases when switching from insecure to secure, BaseURLSource will still include :443 in the URL (hlship: rev 30a66c9526dee3da55a5204e43a169fe4692df79) tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Howard M. Lewis Ship
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development