You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by jf...@apache.org on 2003/12/01 21:44:14 UTC
cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11Processor.java
jfarcand 2003/12/01 12:44:14
Modified: http11/src/java/org/apache/coyote/http11
Http11Processor.java
Log:
Use the socket info instead of hardcoded value. HttpServletRequest.getLocalPort() is currently broken if the port are not the default one.
Revision Changes Path
1.89 +4 -4 jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java
Index: Http11Processor.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java,v
retrieving revision 1.88
retrieving revision 1.89
diff -u -r1.88 -r1.89
--- Http11Processor.java 21 Nov 2003 21:10:21 -0000 1.88
+++ Http11Processor.java 1 Dec 2003 20:44:14 -0000 1.89
@@ -1294,11 +1294,11 @@
if (colonPos < 0) {
if (sslSupport == null) {
- // 80 - Default HTTTP port
- request.setServerPort(80);
+ // Default HTTTP port
+ request.setServerPort(socket.getLocalPort());
} else {
- // 443 - Default HTTPS port
- request.setServerPort(443);
+ // Default HTTPS port
+ request.setServerPort(socket.getLocalPort());
}
request.serverName().setChars(hostNameC, 0, valueL);
} else {
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11Processor.java
Posted by Bill Barker <wb...@wilshire.com>.
----- Original Message -----
From: <jf...@apache.org>
To: <ja...@apache.org>
Sent: Monday, December 01, 2003 12:44 PM
Subject: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
> jfarcand 2003/12/01 12:44:14
>
> Modified: http11/src/java/org/apache/coyote/http11
> Http11Processor.java
> Log:
> Use the socket info instead of hardcoded value.
HttpServletRequest.getLocalPort() is currently broken if the port are not
the default one.
>
-1 for the patch. It breaks NATs, proxies, etc. It also breaks the
servlet-spec for ServletRequest.getServerPort.
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
Hans Bergsten wrote:
> Remy Maucherat wrote:
>
>> [...]
>> Now the big question is actually what the new getLocalPort call should
>> return. If you need an extra localPort (and its friend getLocalName -
>> did you notice the getLocalName() call was always returning whatever
>> hostname was in the host header) field in the request because it
>> doesn't correspond to vhosting (I consider it would be yet another
>> major blunder of the Servlet spec, but what can I do ...), then so be it.
>
> The getLocalXXX() methods are intended to provide information about
> where the request was _recieved_ by the container, rather than where
> it was _sent_ by the client, which is useful information for some types
> of applications, for instance to detect if there's any vhosting
> involved.
So proxying would be detected as vhosting. I think it would indicate if
the server is a pure standalone server, but that's it.
> So you have three sets of methods for getting address/port info.
> * Where the request was received (server socket):
> getLocalName()
> getLocalPort()
> getLocalAddr()
>
> * Where the request was sent (Host header info):
> getServerName()
> getServerPort()
>
> * Where the request comes from (client socket):
> getRemoteName()
> getRemotePort()
> getRemoteAddr()
>
> With an HTTP/1.1 request with a Host header "foo.com" value, proxied
> through Apache to Tomcat listening on port 8005, from a client (or a
> proxy) at 4.62.132.17 (bar.com) port 1766, the methods should return
> these values:
>
> getLocalName() -> localhost (or some other local interface)
> getLocalPort() -> 8005
> getLocalAddr() -> 127.0.0.1 (or some other local interface)
>
> getServerName() -> foo.com
> getServerPort() -> 80
>
> getRemoteName() -> bar.com
> getRemotePort() -> 1766
> getRemoteAddr() -> 4.62.132.17
>
> Without proxying through Apache, a Host header "localhost:8080"
> value and Tomcat listening on port 8080, it would look like this
> instead:
>
> getLocalName() -> localhost
> getLocalPort() -> 8080
> getLocalAddr() -> 127.0.0.1
>
> getServerName() -> localhost
> getServerPort() -> 8080
>
> getRemoteName() -> bar.com
> getRemotePort() -> 1766
> getRemoteAddr() -> 4.62.132.17
This will likely confuse users a bit. If it's as intended, then fine :)
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Jeanfrancois Arcand <jf...@apache.org>.
Hans Bergsten wrote:
> Remy Maucherat wrote:
>
>> [...]
>> Now the big question is actually what the new getLocalPort call
>> should return. If you need an extra localPort (and its friend
>> getLocalName - did you notice the getLocalName() call was always
>> returning whatever hostname was in the host header) field in the
>> request because it doesn't correspond to vhosting (I consider it
>> would be yet another major blunder of the Servlet spec, but what can
>> I do ...), then so be it.
>
>
> The getLocalXXX() methods are intended to provide information about
> where the request was _recieved_ by the container, rather than where
> it was _sent_ by the client, which is useful information for some types
> of applications, for instance to detect if there's any vhosting
> involved.
>
> So you have three sets of methods for getting address/port info.
> * Where the request was received (server socket):
> getLocalName()
> getLocalPort()
> getLocalAddr()
>
> * Where the request was sent (Host header info):
> getServerName()
> getServerPort()
>
> * Where the request comes from (client socket):
> getRemoteName()
> getRemotePort()
> getRemoteAddr()
>
> With an HTTP/1.1 request with a Host header "foo.com" value, proxied
> through Apache to Tomcat listening on port 8005, from a client (or a
> proxy) at 4.62.132.17 (bar.com) port 1766, the methods should return
> these values:
>
> getLocalName() -> localhost (or some other local interface)
> getLocalPort() -> 8005
> getLocalAddr() -> 127.0.0.1 (or some other local interface)
>
> getServerName() -> foo.com
> getServerPort() -> 80
>
> getRemoteName() -> bar.com
> getRemotePort() -> 1766
> getRemoteAddr() -> 4.62.132.17
>
> Without proxying through Apache, a Host header "localhost:8080"
> value and Tomcat listening on port 8080, it would look like this
> instead:
>
> getLocalName() -> localhost
> getLocalPort() -> 8080
> getLocalAddr() -> 127.0.0.1
>
> getServerName() -> localhost
> getServerPort() -> 8080
>
> getRemoteName() -> bar.com
> getRemotePort() -> 1766
> getRemoteAddr() -> 4.62.132.17
Then today's commit implemented that behaviour except for getLocalName(0
which is still broken.Will fix it). Before getLocalPort() was calling
getServerPort() (my mistake back in 03/05). I think the spec should be
clarified with an example like this one ;-)
Thanks for the clarification.
-- Jeanfrancois
>
> HTH,
> Hans
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
Remy Maucherat wrote:
> [...]
> Now the big question is actually what the new getLocalPort call should
> return. If you need an extra localPort (and its friend getLocalName -
> did you notice the getLocalName() call was always returning whatever
> hostname was in the host header) field in the request because it doesn't
> correspond to vhosting (I consider it would be yet another major blunder
> of the Servlet spec, but what can I do ...), then so be it.
The getLocalXXX() methods are intended to provide information about
where the request was _recieved_ by the container, rather than where
it was _sent_ by the client, which is useful information for some types
of applications, for instance to detect if there's any vhosting
involved.
So you have three sets of methods for getting address/port info.
* Where the request was received (server socket):
getLocalName()
getLocalPort()
getLocalAddr()
* Where the request was sent (Host header info):
getServerName()
getServerPort()
* Where the request comes from (client socket):
getRemoteName()
getRemotePort()
getRemoteAddr()
With an HTTP/1.1 request with a Host header "foo.com" value, proxied
through Apache to Tomcat listening on port 8005, from a client (or a
proxy) at 4.62.132.17 (bar.com) port 1766, the methods should return
these values:
getLocalName() -> localhost (or some other local interface)
getLocalPort() -> 8005
getLocalAddr() -> 127.0.0.1 (or some other local interface)
getServerName() -> foo.com
getServerPort() -> 80
getRemoteName() -> bar.com
getRemotePort() -> 1766
getRemoteAddr() -> 4.62.132.17
Without proxying through Apache, a Host header "localhost:8080"
value and Tomcat listening on port 8080, it would look like this
instead:
getLocalName() -> localhost
getLocalPort() -> 8080
getLocalAddr() -> 127.0.0.1
getServerName() -> localhost
getServerPort() -> 8080
getRemoteName() -> bar.com
getRemotePort() -> 1766
getRemoteAddr() -> 4.62.132.17
HTH,
Hans
--
Hans Bergsten <ha...@gefionsoftware.com>
Gefion Software <http://www.gefionsoftware.com/>
Author of O'Reilly's "JavaServer Pages", covering JSP 2.0 and JSTL 1.1
Details at <http://TheJSPBook.com/>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Jeanfrancois Arcand <jf...@apache.org>.
Remy Maucherat wrote:
> Jeanfrancois Arcand wrote:
>
>> Remy Maucherat wrote:
>>
>>> Jeanfrancois Arcand wrote:
>>>
>>>> Remy Maucherat wrote:
>>>>
>>>>> jfarcand@apache.org wrote:
>>>>>
>>>>>> jfarcand 2003/12/01 12:44:14
>>>>>>
>>>>>> Modified: http11/src/java/org/apache/coyote/http11
>>>>>> Http11Processor.java Log: Use the socket info instead of hardcoded
>>>>>> value. HttpServletRequest.getLocalPort() is currently broken if the
>>>>>> port are not the default one.
>>>>>
>>>>>
>>>>> This looks wrong. I am almost certain this was on purpose. These
>>>>> must return the default value for the protocol, not the values
>>>>> actually used by the socket.
>>>>
>>>>
>>>> Humm. Then maybe the way HttpServletRequest.getLocalPort() is
>>>> implemented is wrong. Can socket.getLocalPort() returns something
>>>> different that 80 if the connector supposed to listen on 80 (maybe
>>>> I'm missing something from the jk side here....)
>>>
>>>
>>> I don't remember. I think you should look into the version history.
>>> I will tomorrow.
>>
>>
>> It was added with version 1.23. Bill ? ;-)
>>
>> I've just realized that the sslSupport variable is no longer needed
>> (so my fix needs to be cleaned). BTW the test I'm doing is:
>>
>> telnet localhost 8080
>> GET /tomcat-test/ServletTest HTTP/1.1
>> Host: localhost
>>
>> In the servlet, I'm doing request.getLocalPort(), which always
>> returns 80. But it works if I do
>>
>> telnet localhost 8080
>> GET /tomcat-test/ServletTest HTTP/1.1
>> Host: localhost:8080
>>
>> ;-)
>
>
> Thanks for the example, I now remember the reason for the code. So
> I'll have to veto your patch.
>
> The rule for HTTP/1.1 is that:
> - it should use whatever is specified in the host header
> - it should return the protocol default otherwise, since this is the
> same as not specifying anything in the host header
>
> This comes from vhosting. serverName and serverPort must contain teh
> vhosting information.
>
> HTTP/1.0 doesn't have the host header, so the socket is used.
>
> Now the big question is actually what the new getLocalPort call should
> return. If you need an extra localPort (and its friend getLocalName -
> did you notice the getLocalName() call was always returning whatever
> hostname was in the host header) field in the request because it
> doesn't correspond to vhosting (I consider it would be yet another
> major blunder of the Servlet spec, but what can I do ...), then so be it.
:-( OK I will revert the fix and see what I can do with the Servlet spec
(and try to see if we can do something in another place that will not
impact the current code).
-- Jeanfrancois
>
> Rémy
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
Jeanfrancois Arcand wrote:
> Remy Maucherat wrote:
>
>> Jeanfrancois Arcand wrote:
>>
>>> Remy Maucherat wrote:
>>>
>>>> jfarcand@apache.org wrote:
>>>>
>>>>> jfarcand 2003/12/01 12:44:14
>>>>>
>>>>> Modified: http11/src/java/org/apache/coyote/http11
>>>>> Http11Processor.java Log: Use the socket info instead of hardcoded
>>>>> value. HttpServletRequest.getLocalPort() is currently broken if the
>>>>> port are not the default one.
>>>>
>>>> This looks wrong. I am almost certain this was on purpose. These
>>>> must return the default value for the protocol, not the values
>>>> actually used by the socket.
>>>
>>> Humm. Then maybe the way HttpServletRequest.getLocalPort() is
>>> implemented is wrong. Can socket.getLocalPort() returns something
>>> different that 80 if the connector supposed to listen on 80 (maybe
>>> I'm missing something from the jk side here....)
>>
>> I don't remember. I think you should look into the version history. I
>> will tomorrow.
>
> It was added with version 1.23. Bill ? ;-)
>
> I've just realized that the sslSupport variable is no longer needed (so
> my fix needs to be cleaned). BTW the test I'm doing is:
>
> telnet localhost 8080
> GET /tomcat-test/ServletTest HTTP/1.1
> Host: localhost
>
> In the servlet, I'm doing request.getLocalPort(), which always returns
> 80. But it works if I do
>
> telnet localhost 8080
> GET /tomcat-test/ServletTest HTTP/1.1
> Host: localhost:8080
>
> ;-)
Thanks for the example, I now remember the reason for the code. So I'll
have to veto your patch.
The rule for HTTP/1.1 is that:
- it should use whatever is specified in the host header
- it should return the protocol default otherwise, since this is the
same as not specifying anything in the host header
This comes from vhosting. serverName and serverPort must contain teh
vhosting information.
HTTP/1.0 doesn't have the host header, so the socket is used.
Now the big question is actually what the new getLocalPort call should
return. If you need an extra localPort (and its friend getLocalName -
did you notice the getLocalName() call was always returning whatever
hostname was in the host header) field in the request because it doesn't
correspond to vhosting (I consider it would be yet another major blunder
of the Servlet spec, but what can I do ...), then so be it.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Jeanfrancois Arcand <jf...@apache.org>.
Remy Maucherat wrote:
> Jeanfrancois Arcand wrote:
>
>> Remy Maucherat wrote:
>>
>>> jfarcand@apache.org wrote:
>>>
>>>> jfarcand 2003/12/01 12:44:14
>>>>
>>>> Modified: http11/src/java/org/apache/coyote/http11
>>>> Http11Processor.java Log: Use the socket info instead of hardcoded
>>>> value. HttpServletRequest.getLocalPort() is currently broken if the
>>>> port are not the default one.
>>>
>>>
>>> This looks wrong. I am almost certain this was on purpose. These
>>> must return the default value for the protocol, not the values
>>> actually used by the socket.
>>
>>
>> Humm. Then maybe the way HttpServletRequest.getLocalPort() is
>> implemented is wrong. Can socket.getLocalPort() returns something
>> different that 80 if the connector supposed to listen on 80 (maybe
>> I'm missing something from the jk side here....)
>
>
> I don't remember. I think you should look into the version history. I
> will tomorrow.
It was added with version 1.23. Bill ? ;-)
I've just realized that the sslSupport variable is no longer needed (so
my fix needs to be cleaned). BTW the test I'm doing is:
telnet localhost 8080
GET /tomcat-test/ServletTest HTTP/1.1
Host: localhost
In the servlet, I'm doing request.getLocalPort(), which always returns
80. But it works if I do
telnet localhost 8080
GET /tomcat-test/ServletTest HTTP/1.1
Host: localhost:8080
;-)
-- Jeanfrancois
>
>> Also we are doing something similar if http 1.0 is used (just above):
>>
>>> // HTTP/1.0
>>> // Default is what the socket tells us. Overriden if a
>>> host is
>>> // found/parsed
>>> request.setServerPort(socket.getLocalPort());
>>
>
> It goes both ways. One possible explanation for that is that this code
> is much less used, and it never got fixed.
>
> Remy
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
Jeanfrancois Arcand wrote:
> Remy Maucherat wrote:
>
>> jfarcand@apache.org wrote:
>>
>>> jfarcand 2003/12/01 12:44:14
>>>
>>> Modified: http11/src/java/org/apache/coyote/http11
>>> Http11Processor.java Log: Use the socket info instead of hardcoded
>>> value. HttpServletRequest.getLocalPort() is currently broken if the
>>> port are not the default one.
>>
>> This looks wrong. I am almost certain this was on purpose. These must
>> return the default value for the protocol, not the values actually
>> used by the socket.
>
> Humm. Then maybe the way HttpServletRequest.getLocalPort() is
> implemented is wrong. Can socket.getLocalPort() returns something
> different that 80 if the connector supposed to listen on 80 (maybe I'm
> missing something from the jk side here....)
I don't remember. I think you should look into the version history. I
will tomorrow.
> Also we are doing something similar if http 1.0 is used (just above):
>
>> // HTTP/1.0
>> // Default is what the socket tells us. Overriden if a
>> host is
>> // found/parsed
>> request.setServerPort(socket.getLocalPort());
It goes both ways. One possible explanation for that is that this code
is much less used, and it never got fixed.
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Jeanfrancois Arcand <jf...@apache.org>.
Remy Maucherat wrote:
> jfarcand@apache.org wrote:
>
>> jfarcand 2003/12/01 12:44:14
>>
>> Modified: http11/src/java/org/apache/coyote/http11
>> Http11Processor.java Log: Use the socket info instead of hardcoded
>> value. HttpServletRequest.getLocalPort() is currently broken if the
>> port are not the default one.
>
>
> This looks wrong. I am almost certain this was on purpose. These must
> return the default value for the protocol, not the values actually
> used by the socket.
Humm. Then maybe the way HttpServletRequest.getLocalPort() is
implemented is wrong. Can socket.getLocalPort() returns something
different that 80 if the connector supposed to listen on 80 (maybe I'm
missing something from the jk side here....)
Also we are doing something similar if http 1.0 is used (just above):
> // HTTP/1.0
> // Default is what the socket tells us. Overriden if a
> host is
> // found/parsed
> request.setServerPort(socket.getLocalPort());
-- Jeanfrancois
>
> Remy
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11Processor.java
Posted by Remy Maucherat <re...@apache.org>.
jfarcand@apache.org wrote:
> jfarcand 2003/12/01 12:44:14
>
> Modified: http11/src/java/org/apache/coyote/http11
> Http11Processor.java Log: Use the socket info instead of hardcoded
> value. HttpServletRequest.getLocalPort() is currently broken if the
> port are not the default one.
This looks wrong. I am almost certain this was on purpose. These must
return the default value for the protocol, not the values actually used
by the socket.
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org