On Wed, Oct 21, 2015 at 5:12 AM, Daniel Dekany <[email protected]> wrote:
> Tuesday, October 20, 2015, 6:08:33 AM, Woonsan Ko wrote:
>
>> Hi Daniel,
>>
>> Thanks a lot for the reasonable/thorough analysis and solutions!
>> Yes, your proposal will solve our problem (FREEMARKER-1). We can
>> probably set it to "never" in our default configuration.
>>
>> In our case, our MVC framework allows developers to set response
>> content type to anything before dispatching to freemarker servlet. For
>> example, they can set "text/html", "application/xml",
>> "application/json", "application/vnd.api+json", or whatever they want
>> to use. In that case, some developers prefers setting a content type
>> in a controller with setting model beans (which can be written as
>> string, html, xml or json string) in request attributes before
>> dispatching to freemarker servlet.
>> I understand it can be solved in different ways, but I'd like to
>> support them with what they're allowed to by servlet specification.
>> Your proposal with the different options seems fulfilling this.
>
> OK, so that's how it will be, unless somebody else says something.
>
> We may also need a protected getDefaultOverrideRequestContentType()
> method, so that a subclass can change the default from "always".
That would be nice!
>
> Any comments about the output charset selection? How do you at Hippo
> control that?
Mostly they have been okay with the default ContentType configuration
as init param: "text/html; charset=UTF-8". This default configuration
has fulfilled most of our use cases even if they want to control it in
controllers in some cases. Also probably they are okay with UTF-8
encoding in almost every case.
All those options ("fromTemplate", "always ${charset}" and "doNotSet")
seem like good options to choose. Personally speaking, due to nature
of our HMVC (with which the top ancestor controller's template's
contentType setting will win), I normally recommend our customers to
set it globally, not from template in most cases, just to avoid
confusions from the controller component hierarchy.
>
>> By the way, regarding FREEMARKET-2, maybe do we need to have
>> somethings similar for FreemarkerServlet#deduceLocale() with
>> "OverrideResponseLocale" init-param? e.g, "always" and "never".
>> "never" behaves like "always" if HttpServletRequest#getLocale()
>> returns null.
>
> So, OverrideResponseLocale "never" wouldn't even call deduceLocale()
> if servletRequest.locale is non-null. Otherwise we call deduceLocale()
> and use what it returns. A bit twisted in that an overridden
> deduceLocale() could do all this alone (examine servletRequest.locale,
> decide if it will override it), but I guess this is the most practical
> compromise.
OK. Sounds great! Thanks for clarification! I'll try to create a PR
for this soon.
>
> Note that for any of these init-params, even where right now we only
> have two possible values, I prefer a string value over a boolean
> because it's more future proof.
Makes sense.
Thanks a lot!
Cheers,
Woonsan
>
> --
> Thanks,
> Daniel Dekany
>
>> Regards,
>>
>> Woonsan
>>
>>
>> On Mon, Oct 19, 2015 at 4:28 AM, Daniel Dekany <[email protected]> wrote:
>>> So I have merged this
>>> (https://issues.apache.org/jira/browse/FREEMARKER-1) in, but while
>>> reviewing the stuff, I have realized that I'm not sure how exactly
>>> this should work, especially as OutputFormat-s (the main 2.3.24
>>> feature, mostly used for auto-escaping, but usually also specifies a
>>> MIME type) comes into the picture. As far as I see, the most generic
>>> solution would be this:
>>>
>>> The new "OverrideResponseContentType" FreemarkerServlet init-param
>>> (the topic of https://issues.apache.org/jira/browse/FREEMARKER-1)
>>> could have three values:
>>>
>>> - "always": We always set it to something (see later to what),
>>> overriding and ignoring anything that was in
>>> servletResponse.contentType. This is the backward compatible mode,
>>> and so the default. I also believe that this happens to be good
>>> default regardless of backward compatibility, because it's the
>>> business of the MVC View to decide such things as the MIME type.
>>>
>>> - "never": Never *overrides*, means if servletResponse.contentType
>>> it null, it will still set it, like "always" does, otherwise it
>>> just leaves servletResponse.contentType alone. This means that the
>>> code that forwards to FreemarkerSerlvet has full control over the
>>> servletResponse.contentType, if it cares to set it.
>>>
>>> - "whenTemplateHasContentType": If either the legacy "content_type"
>>> custom template attribute is present (usually set with <#ftl
>>> attributes={"content_type": "..."}>), or the template has an
>>> associated OutputFormat that specifies a non-null MIME type (usually
>>> "text/html"), then we behave as "always", otherwise as "never". The
>>> logic behind this is that if a template has an associated MIME Type,
>>> then that's certainly correct and shouldn't trampled on.
>>> Applications may will mix legacy templates with those with
>>> associated OutputFormat, that's why the "never" branch has
>>> importance.
>>>
>>> So what does "always" set the content type to? We have a "ContentType"
>>> FreemarkerServlet init-param since forever (defaulting to
>>> "text/html"), and most often that's what we set it to. There's also an
>>> old and obscure feature where you can specify a different one per
>>> template via the "content_type" custom template attribute. But the
>>> important new thing is that associating OutputFormat to each template
>>> meant to become the norm in the future, and an OutputFormat usually
>>> specifies a MIME type too. So we have a non-obscure mechanism to
>>> figure out the content type for an *individual* template, starting
>>> from 2.3.24. (Of course most applications won't set up OutputFormat
>>> associations for a long time, and in that case templates are
>>> associated to the "undefined" OutputFormat, that specifies no MIME
>>> type, so we just get the old behavior.)
>>>
>>> I have also realized that the way one can specify the output charset
>>> with FreemarkerServlet is crude too. So I'm thinking about adding an
>>> "ResponseCharacterEncoding" FreemarkerServlet init-param too. There
>>> the most generic set of values would look like this:
>>>
>>> - "legacy": Default for backward compatibility. Has some strange
>>> quirks... It doesn't mater how it works now, as it's a legacy.
>>>
>>> - "fromTemplate": We get the charset from template.getOutputEncoding(),
>>> which is an optional setting existing for a good while. If that's
>>> null, we use template.getEncoding(), which is the encoding of the
>>> source file.
>>>
>>> - "always ${charset}", like "always UTF-8": We just always set it to that,
>>> via ServletResponse.setCharacterEncoding.
>>>
>>> - "doNotSet": We just don't set it, ever. (We can't detect if it was
>>> set in the ServletRequest by the forwared, so we can't be smarter, I
>>> believe.)
>>>
>>> A side note is that the modes other than "legacy" will need at least
>>> Servlet 2.4, so that we will have
>>> ServletResponse.setCharacterEncoding, instead of wresting with
>>> appending it the charset to the contentType.
>>>
>>> Comments, ideas? How does this fit your applications?
>>>
>>> --
>>> Thanks,
>>> Daniel Dekany
>>>
>>>
>>>
>>> Sunday, October 11, 2015, 4:43:13 AM, Woonsan Ko wrote:
>>>
>>>> Hi,
>>>>
>>>> I've just created a PR for FREEMARKER-1:
>>>> - https://github.com/apache/incubator-freemarker/pull/1
>>>>
>>>> I suppose that PR should be automatically posted here soon.
>>>>
>>>> Just for information, PRs on the github mirror can be applied in the
>>>> way as explained in the "Applying Pull Requests (for git based
>>>> components)" section [1].
>>>>
>>>> Cheers,
>>>>
>>>> Woonsan
>>>>
>>>> [1] https://wiki.apache.org/commons/UsingGIT
>>>>
>>>>
>>>> On Fri, Oct 9, 2015 at 6:52 AM, Woonsan Ko <[email protected]> wrote:
>>>>> Hi folks,
>>>>>
>>>>> I've just created two JIRA issues:
>>>>> - https://issues.apache.org/jira/browse/FREEMARKER-1
>>>>> - https://issues.apache.org/jira/browse/FREEMARKER-2
>>>>>
>>>>> Yeah, number 1 and 2! :)
>>>>>
>>>>> Those were actually discussed in the old ML. I contracted those to the
>>>>> two issues.
>>>>> Hopefully I can create PRs and ask for review sooner or later.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Woonsan
>>>>
>>>
>>
>
