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".

Any comments about the output charset selection? How do you at Hippo
control that?

> 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.

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.

-- 
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
>>>
>>
>

Reply via email to