Tapestry 5
  1. Tapestry 5
  2. TAP5-1973

:443 added to URLs when using the Link.toAbsoluteURI(true)

    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.

        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 ]

          People

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

            Dates

            • Created:
              Updated:

              Development