Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MODCLUSTER-798 Fix handling of offseted ports #735

Closed
wants to merge 2 commits into from

Conversation

jajik
Copy link
Collaborator

@jajik jajik commented Sep 25, 2023

https://issues.redhat.com/browse/MODCLUSTER-798

This PR makes it work when portOffset for tomcats 8.5 and 9.0. No fix is required for tomcat-10.1 which seems strange, however, I was not able to determine why tomcat-10.1 behaves differently. Because of this I believe the fix can be made better by someone who understand the codebase better.

This is the server.xml I used

<?xml version="1.0" encoding="UTF-8"?>
<Server port="8005" shutdown="SHUTDOWN" address="127.0.0.1" portOffset="port_offset">
  <Listener className="org.jboss.modcluster.container.catalina.standalone.ModClusterListener"
              connectorPort="8080"
              advertise="false"
              stickySession="true"
              stickySessionForce="false"
              stickySessionRemove="true"
              proxyList="127.0.0.1:6666" />
  <Listener className="org.apache.catalina.startup.VersionLoggerListener" />
  <Listener className="org.apache.catalina.core.AprLifecycleListener" SSLEngine="on" />
  <Listener className="org.apache.catalina.core.JreMemoryLeakPreventionListener" />
  <Listener className="org.apache.catalina.mbeans.GlobalResourcesLifecycleListener" />
  <Listener className="org.apache.catalina.core.ThreadLocalLeakPreventionListener" />

  <GlobalNamingResources>
    <Resource name="UserDatabase" auth="Container"
              type="org.apache.catalina.UserDatabase"
              description="User database that can be updated and saved"
              factory="org.apache.catalina.users.MemoryUserDatabaseFactory"
              pathname="conf/tomcat-users.xml" />
  </GlobalNamingResources>

  <Service name="Catalina">
    <Connector port="8080" protocol="HTTP/1.1"
               address="127.0.0.1"
               connectionTimeout="20000"
               redirectPort="8443"
               portOffset="port_offset" />
    
    <Connector protocol="AJP/1.3"
               address="127.0.0.1"
               port="8009"
               secretRequired="false"
               redirectPort="8443"
               portOffset="port_offset" />
   
    <Engine name="Catalina" defaultHost="localhost">

      <Realm className="org.apache.catalina.realm.LockOutRealm">
        <Realm className="org.apache.catalina.realm.UserDatabaseRealm"
               resourceName="UserDatabase"/>
      </Realm>

      <Host name="localhost"  appBase="webapps"
            unpackWARs="true" autoDeploy="true">

        <Valve className="org.apache.catalina.valves.AccessLogValve" directory="logs"
               prefix="localhost_access_log" suffix=".txt"
               pattern="%h %l %u %t &quot;%r&quot; %s %b" />
      </Host>
    </Engine>
  </Service>
</Server>

with tomcat1 running on 127.0.0.1:8080 (offset 0) and tomcat2 on 127.0.0.1:8081 (offset 1). Without these changes, proxy gets CONFIG message for tomcat2 with port 8080. Alternatively, I could change connectorPort for tomcat2 to 8081, but that would cause some issues in createProxyConnector function (e.g., connectorPort.equals(connector.getPort())).

Edit: Fix formatting.

@rhusar
Copy link
Member

rhusar commented Sep 25, 2023

Looks like portOffset is not widely used setting since noone reported this bug before. Anyway, I ll have a look, but the general fix looks about right.

@jajik
Copy link
Collaborator Author

jajik commented Sep 25, 2023

Thanks a lot! (I would be more confident about the fix if tomcat-10.1 would require it too.)

@rhusar rhusar self-assigned this Sep 25, 2023
@jajik jajik force-pushed the main branch 2 times, most recently from 66f0376 to 689f3c3 Compare October 11, 2023 13:02
@jajik jajik force-pushed the main branch 2 times, most recently from f2e9b34 to e4eab59 Compare October 20, 2023 10:13
@jajik jajik force-pushed the main branch 2 times, most recently from fe8372f to 1f4c113 Compare November 16, 2023 11:54
@rhusar
Copy link
Member

rhusar commented Dec 12, 2023

Thanks @jajik - I could reproduce this with Tomcat 10 no problem. That needs fixing too - replaced by #762

@rhusar rhusar closed this Dec 12, 2023
@jajik
Copy link
Collaborator Author

jajik commented Dec 12, 2023

Ok, glad to hear that, now it makes sense 🙂

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.

2 participants