Re: Re: svn commit: r1817310 - /turbine/core/trunk/src/java/org/apache/turbine/Turbine.java

[Georg Kallidis <ge...@cedis.fu-berlin.de>]

Hi,
as this is an ongoing issue (and the last obstacle before upgrading to 
Turbine 4.1) I just want to sum up before committing any solutions.
> The DefaultTurbineRunData.getDefaultCharSet() 
> currently pulls from Turbine configuration constants, and without the 
> current patch, this value never gets updated by the user defined 
property.
What's intended?
- input.encoding is used for is setting req.setCharacterEncoding, if the 
(tomcat-)container does not already set this.
- locale.default.charset is on the other side used (only) in rundata's 
getDefaultCharSet method to define the charset in getContentType, if the 
content type matches text and in getCharSet - called once when hit first 
time. "Default" does in this context just mean a Turbine configured 
default, not any hard-coded default. The locale based mimetype charset 
check and set in this method could never be called IMO and may be better 
moved to another place. It may be (re)used only optionally e.g. by setting 
services.VelocityService.default.page to a class extending DefaultPage 
overriding the method doBuildBeforeAction (calling rundata.setCharSet, per 
request, executed in ExecutePageValve). As a result the charSet field 
value in rundata is never set, which might be a problem, when the content 
type does not match "text/".

As a result, the first step would be IMO, that we just remove the line in 
Turbine.java (input.encoding overriding locale.default.charset) and check 
(together), why in the aforementioned context "this value never gets 
updated".
E.g. in https://wiki.apache.org/tomcat/FAQ/CharacterEncoding it is 
mentioned, that 
"In Tomcat 8 starting with 8.0.0 (8.0.0-RC3, to be specific), the default 
value of URIEncoding attribute on the <Connector> element depends on 
"strict servlet compliance" setting. The default value (strict compliance 
is off) of URIEncoding is now UTF-8. If "strict servlet compliance" is 
enabled, the default value is ISO-8859-1. 

Best regards, Georg



Von:    Jeffery Painter <je...@jivecast.com>
An:     dev@turbine.apache.org
Datum:  11.12.2017 19:01
Betreff:        Re: svn commit: r1817310 - 
/turbine/core/trunk/src/java/org/apache/turbine/Turbine.java



Hi Thomas,

It sounds like you recommending then that we get rid of 
input.encoding=<user defined> completely from the TR.props config. Is 
that correct? It has been in there since 2.3.x days

I searched through the code base and the method call to 
Turbine.getDeafultInputEncoding() is never referenced anywhere currently 
except in one test case.  The DefaultTurbineRunData.getDefaultCharSet() 
currently pulls from Turbine configuration constants, and without the 
current patch, this value never gets updated by the user defined property.

It would make more sense to me that it should be handled as you mention 
below, but I am not sure where to start to make all the fixes required. 
Similar issues exist around updating the default locale.

If we were to get rid of it completely, do you agree UTF-8 should be the 
new default or are there reasons it should be something else? How do we 
handle modifying the default locale to also conform?

I could not move to Turbine 4.0 without UTF-8 support, and this fix 
allows for it.  Seems a bit clunky to have to set it manually the way I 
have done though.  Open to your thoughts/suggestions on a better way 
forward :-)

Thanks,
Jeff

On 12/08/2017 10:52 AM, Thomas Vandahl wrote:
> On 06.12.17 17:51, painter@apache.org wrote:
>> Author: painter
>> Date: Wed Dec  6 16:51:52 2017
>> New Revision: 1817310
>>
>> URL: http://svn.apache.org/viewvc?rev=1817310&view=rev
>> Log:
>> Set the configuration parameter to honor the value given by the user in 
TR.props
> -1
>
> This change will only cause confusion.
>
> We should remove the configuration setting then completely (as you will
> be overwriting it in any case) and rely on
> Turbine.getDefaultInputEncoding() only (making this static).
>
> Bye, Thomas
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@turbine.apache.org
> For additional commands, e-mail: dev-help@turbine.apache.org
>
>

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

CharacterEncoding, was: Re: svn commit: r1817310 - /turbine/core/trunk/src/java/org/apache/turbine/Turbine.java

[Thomas Vandahl <tv...@apache.org>]

Hi folks,

On 12.04.18 15:58, Georg Kallidis wrote:
> default, not any hard-coded default. The locale based mimetype charset 
> check and set in this method could never be called IMO and may be better 
> moved to another place. It may be (re)used only optionally e.g. by setting 
> services.VelocityService.default.page to a class extending DefaultPage 
> overriding the method doBuildBeforeAction (calling rundata.setCharSet, per 
> request, executed in ExecutePageValve). As a result the charSet field 
> value in rundata is never set, which might be a problem, when the content 
> type does not match "text/".

I suggest to move the encoding/charset handling to a valve. That way,
you can switch it on and off or change it as required.

See my commit in r1836928. WDYT?

Bye, Thomas

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