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

          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
          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
          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.
          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.
          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 -

          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.
          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?
          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
          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.

            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