Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Make wicket encode its fortunes response in UTF-8#2662

Closed
michaelhixson wants to merge 1 commit intoTechEmpower:masterfrom
michaelhixson:fix-wicket-fortunes-encoding
Closed

Make wicket encode its fortunes response in UTF-8#2662
michaelhixson wants to merge 1 commit intoTechEmpower:masterfrom
michaelhixson:fix-wicket-fortunes-encoding

Conversation

@michaelhixson
Copy link
Copy Markdown
Contributor

This should repair the wicket fortune test on ServerCentral, which is currently failing.

It appeared to be using the system default charset instead, which was not necessarily UTF-8.

Changing the TEXT_HTML constant to "text/html;charset=utf-8" also would have fixed it, but this solution involves fewer lines of code and seems (to me) like more idiomatic wicket application code.

I removed the renderXmlDecl() override for a similar reason (fewer lines, seems more idiomatic) although it had nothing to do with the UTF-8 issue.

I removed the initialCapacity argument for the ArrayList because there's no good reason to set it to 10,000. Since the test requirements state that the list shouldn't be sized using foreknowledge of the row count, we might as well trust ArrayList's defaults.

It appeared to be using the system default charset instead, which was
not necessarily UTF-8.

Changing the TEXT_HTML constant to "text/html;charset=utf-8" also would
have fixed it, but this solution involves fewer lines of code and seems
(to me) like more idiomatic wicket application code.

I removed the renderXmlDecl() override for a similar reason (fewer lines,
seems more idiomatic) although it had nothing to do with the UTF-8 issue.

I removed the initialCapacity argument for the ArrayList because there's
no good reason to set it to 10,000.  Since the test requirements state
that the list shouldn't be sized using foreknowledge of the row count,
we might as well trust ArrayList's defaults.
@martin-g
Copy link
Copy Markdown
Contributor

martin-g commented Apr 5, 2017

Changing the TEXT_HTML constant to "text/html;charset=utf-8" also would have fixed it, but this solution involves fewer lines of code and seems (to me) like more idiomatic wicket application code.

Idiomatic code is also to use something like response.write("Hello world") but because these are performance related tests many frameworks use optimizations like using byte[] or ByteBuffer.

The changes you revert in this PR are done intentionally to reduce the response size to the required minimum.

About ArrayList size: I don't remember any requirements about its initial size, but I also do not re-read the requirements very often.
The default is java.util.ArrayList#DEFAULTCAPACITY_EMPTY_ELEMENTDATA, an empty array, i.e. it may need to do 14 Array.copyOf() calls in the worst case (2^14). But if this is against the rules then fine!

Please do not remove renderXmlDecl(). As I said other tests are also not idiomatic.

@michaelhixson
Copy link
Copy Markdown
Contributor Author

Ok, I'll make a new PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants