Timothy Potter
added a comment - 15/Jul/14 21:23 I'll take a look but I think the fix should be upstream from the managed resource implementations, seems like Restlet should have already done the decoding?

ASF GitHub Bot
added a comment - 25/Jul/14 12:53 GitHub user timoschmidt opened a pull request:
https://github.com/apache/lucene-solr/pull/73
SOLR-6163 : special chars and ManagedSynonymFilterFactory
Special characters could not be used for update or deletion because the url was not decoded before the resource was used.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/timoschmidt/lucene-solr origin/branch_4x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/73.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #73
commit 0168e160e4a9236b047b2e24909d1f59dfd3eb7b
Author: timo.schmidt <timo-schmidt@gmx.net>
Date: 2014-07-25T12:44:26Z
SOLR-6163 : special chars and ManagedSynonymFilterFactory

There are lots of similar "Note that no URI decoding is done by this method." and "Returns the optionnally decoded ______" combinations in the Request class – we should probably audit all of our usages of this class.

Hoss Man
added a comment - 25/Jul/14 21:26 A quick glance at timo's patch and the javadocs for the assocaited restlet classes seems to suggest that this is correct general course of action...
http://restlet.com/learn/javadocs/2.1/jse/api/org/restlet/data/Reference.html#getPath%28%29
"Note that no URI decoding is done by this method. "
A cleaner fix is probably to use this alternative restlet method that an decode for us ...
http://restlet.com/learn/javadocs/2.1/jse/api/org/restlet/data/Reference.html#getPath%28boolean%29
There are lots of similar "Note that no URI decoding is done by this method." and "Returns the optionnally decoded ______" combinations in the Request class – we should probably audit all of our usages of this class.

Hi Vitaly, Thanks for posting a patch ... looks good, except I think we should add some specific tests for this issue in the TestManagedStopFilterFactory and TestManagedSynonymFilterFactory for two reasons: 1) to guard against regression in case something in the RestManager layer changes and 2) to serve as an example to remind developers to test with data requiring decoding when developing test cases for new managed resources.

Timothy Potter
added a comment - 28/Jul/14 16:03 Hi Vitaly, Thanks for posting a patch ... looks good, except I think we should add some specific tests for this issue in the TestManagedStopFilterFactory and TestManagedSynonymFilterFactory for two reasons: 1) to guard against regression in case something in the RestManager layer changes and 2) to serve as an example to remind developers to test with data requiring decoding when developing test cases for new managed resources.

Timo Hund
added a comment - 29/Jul/14 09:53 Hello together,
I've added a specific test for special characters and added a new patch+commited the changes to the pull request.
If you need further changes please let me know