CAM-10472 fix(rest) URLEncoder.encode Content-Disposition filename to support UTF-8 encoded filenames

[ATropichev]

Solving the problem with downloading a file from a File type variable when non-ASCII characters are used in the file name

related to CAM-10472

[CLAassistant]

All committers have signed the CLA.

[tasso94]

Hi @ATropichev,

Thank you for your pull request.

I'll have a look soon.

Stay tuned!

Best,
Tassilo

[tasso94]

Hi @ATropichev,

Thank you for your pull request to support UTF-8 characters in filenames.

There are much more places where this change would make sense:

• HistoricProcessInstanceRestServiceImpl
• CaseDefinitionResourceImpl
• DecisionDefinitionResourceImpl
• DecisionRequirementsDefinitionResourceImpl
• DeploymentResourcesResourceImpl
• ProcessDefinitionResourceImpl
• TaskReportResourceImpl

Would you mind fixing it there as well?

Best,
Tassilo

[ATropichev]

Hi @ATropichev,

Thank you for your pull request to support UTF-8 characters in filenames.

There are much more places where this change would make sense:

* HistoricProcessInstanceRestServiceImpl

* CaseDefinitionResourceImpl

* DecisionDefinitionResourceImpl

* DecisionRequirementsDefinitionResourceImpl

* DeploymentResourcesResourceImpl

* ProcessDefinitionResourceImpl

* TaskReportResourceImpl

Would you mind fixing it there as well?

Best,
Tassilo

Hi, Tassilo,
Thank you for your attention.
I need to look at this
and I can try

Regards,
Andrei

[tasso94]

Hi @ATropichev,

Are you still interested in working on fixing the problem at the other places?

Best,
Tassilo

[ATropichev]

Hi @ATropichev,

Are you still interested in working on fixing the problem at the other places?

Best,
Tassilo

Hi Tassilo,

Of course. I'll try to finish this week.

Regards,
Andrei

[ATropichev]

Hi Tassilo,

Here is my next attempt to fixing this issue

Regards,
Andrei

[tasso94]

Hi @ATropichev,

Looks good to me. However, there were 47 failing tests when I ran the unit tests since the assertions were not adjusted.

Could you do this as well?

Here is a list of the test classes that need adjustments:

Best,
Tassilo

[ATropichev]

Hi Tassilo

Looks good to me. However, there were 47 failing tests when I ran the unit tests since the assertions were not adjusted.

Could you do this as well?

Yeah. I'll try to complete it this week.

Regards,
Andrei

[ATropichev]

Hi Tassilo,

I fixed my bug in URLEncodingUtil class (added support for null value) and edited the tests. All is ready.

Regards,
Andrei

[tasso94]

Hi @ATropichev,

Thank you! I'll have a look into it this Friday. 🙂 👍

Best,
Tassilo

[tasso94]

Hi @ATropichev,

Looks good to me in general, and our test suite passes! However, I have one more change request:

Currently, the header looks as follows when downloading a file with the filename "naïve with umläut.txt":

Content-Disposition: attachment; filename*=UTF-8"na%C3%AFve%20with%20uml%C3%A4ut.txt"

According to RFC 5987, it should look as follows:

Content-Disposition: attachment; filename*=UTF-8''na%C3%AFve%20with%20uml%C3%A4ut.txt

Please note the single quotes after the charset '' and no embracing double-quotes.

We should also keep the legacy filename attribute for backward compatibility with browsers that don't support filename*. Like this, the browser can choose the one it provides support for.

Example:

Content-Disposition: attachment; filename="naïve with umläut.txt"; filename*=UTF-8''na%C3%AFve%20with%20uml%C3%A4ut.txt

To avoid duplicating the string at multiple places, maybe you can add a method to your URLEncodingUtil with filename and filenameEncoded parameters that return the string. Also, when your work with java.text.MessageFormat.format("attachment; filename=\"{0}\"; filename*=UTF-8''{1}", fileName, fileNameEncoded) it becomes even more readable.

You can find an example reflecting your code changes below ⬇️

Best,
Tassilo

[ATropichev]

Hi Tassilo,

I rewrite my URLEncodingUtil class (added buildAttachmentValue method) and edited the tests. All done.

Regards,
Andrei

[tasso94]

Hi @ATropichev,

Thank you for applying my review hints. Looks almost perfect now 🙂 👍

I adjusted URLEncodingUtil slightly to comply with our coding style guide. Also, I changed the code to avoid throwing an exception. Instead, the passed value is returned as-is if an UnsupportedEncodingException occurs.

I've merged your contribution with our codebase. 🚀

Thanks a lot!

Best,
Tassilo