tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: svn commit: r745842 - in /tomcat/connectors/trunk/jk: native/apache-2.0/ native/common/ xdocs/miscellaneous/
Date Wed, 25 Feb 2009 13:26:36 GMT
On 25.02.2009 13:44, Mladen Turk wrote:
> Rainer Jung wrote:
>>> + if (aw->addr_sequence != aw->s->addr_sequence) {
>>> + aw->addr_sequence = aw->s->addr_sequence;
>>> + aw->host = aw->s->hostname;
>>
>> Should we copy the string here?
>>
>
> Nope, the string is now in shared memory.
> In theory we don't care about ajp->host
> we just need that for a resolve, so actually
> it will always point to the same shm address.

I didn't yet go through all the address changes in detail, but I somehow 
like the idea, that the worker itself represents the data that is used 
for forwarding, and the shm represents the most recent status worker 
changes. Of course we try to keep the time gap between the two as small 
as possible, but I think it would be cleaner to keep the two things apart.

What do you think?

Concerning host vs. hostname: I would prefer to keep the variable names 
the same. I can do the change. Do you prefer host in both cases, or 
hostname? I think host makes somes sense, because it reflects the 
workers.properties attribute.

>>> + aw->port = aw->s->port;
>>> + if (!jk_resolve(aw->host, aw->port,&aw->worker_inet_addr,
>>> + aw->worker.we->pool, l)) {
>>> + if (is_error)
>>> + *is_error = JK_HTTP_SERVER_ERROR;
>>
>> i think we should log an error in this case and maybe also reset the
>> data to the previous working set.
>>
>
> I don't like resetting to a previous version.
> If it's bad, it's bad, just like with init.

Hmm, but that means any typo when submitting the status form will break 
the application. Of course we can't fix this in any case, but at least 
if the address doesn't resolve, we could. Maybe we should add a 
resolution test when the form gets submitted, and if the address doesn't 
resolve, we immediately return an error message in the status worker. 
Are you OK with that? I would add it then. It is kind of validation of 
form input data.

In any case a log message (error) is needed, because the problem is fatal.

>>> + if (!p->host) {
>>> + p->host = "undefined";
>>> + }
>>
>> Guess we don't need that, because jk_get_worker_host() already gets a
>> default as the last argument (usually localhost).
>>
>
> True unless the previous host was null as well.

the code at the moment sets it either to AJP13_DEF_HOST (=localhost) or 
to AJP14_DEF_HOST (=localhost) or jumps out of the function before 
reaching the above lines.

>>> - jk_log(l, JK_LOG_ERROR,
>>> - "invalid host and port %s %d",
>>> - ((p->host == NULL) ? "NULL" : p->host), p->port);
>>
>> It was an error log message, now it is debug. Why don't we simpy allow
>> ports below 1024?
>>
>
> We could. Nothing prevents that.

Any idea, why this port limit was chosen? What's bad about allowing to 
connect to a privileged port?

Regards,

Rainer

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message