search

martin-magakian / Amazing-Cloud-Search

Allow you to search, faceted search, add, update, remove objects from your Amazon Cloud Search Index in C#.
30 stars 17 forks source link

URL encode data used in StringBooleanCondition. #12

Closed alyssa-dahlberg closed 10 years ago

alyssa-dahlberg commented 10 years ago

Heya, this one came up when we tried to query with things like & in the condition. Is this the correct place to be encoding this data?

martin-magakian commented 10 years ago

Hi Barry, Nice to see an other commit from you.

I'm been thinking about this small change lately. It look right to me but I have a bad feeling about the URLEncoding.

Do you think any request can work with EscapeDataString? http://jasonrowe.com/2008/07/29/decrypt-querystring-space-issue-uriescapedatastring/ EscapeDataString change into: & ----> %26 $ ----> %24

So all my concerne is to know if this commit can affect anyone ?

alyssa-dahlberg commented 10 years ago

Hi Martin,

My understanding is that AWS should be doing the opposite (UrlDecode equivalent) before processing the search. My testing suggests this is the case. If you take the follow evil search string:

Test %20 spaces'&"other\fun $+uff.

It encodes to this in the URL

Test%20%2520%20spaces%27%26%22other%5Cfun%20%24%2Buff.

When you submit it to Cloud Search the response (raw JSON) includes this summary of the query, which suggests it is being decoded correctly:

"match-expr":"(label 'Test %20 spaces\\'&\"other\\\\fun $+uff.')"

If we don't encode the query things like "&" confuse the query into doing Unexpected Things. Might be worth a review of where else we should be encoding things, I haven't got time just at the moment though sorry.

martin-magakian commented 10 years ago

Thanks for your research.

I validated your push request. I also had a quick review. It look like we was doing the same error in "StringListBooleanCondition" as well as "StringFacetConstraints"

I just fixed it: https://github.com/martin-magakian/Amazing-Cloud-Search/commit/852142d658a0c04ce4590ad089e2dcd4ac5751f6

Let me know if you see any mistake in this commit.

alyssa-dahlberg commented 10 years ago

Looks fine to me :+1:

kingdango commented 10 years ago

Nice work dude! Cheers!

-Aaron

On Sun, Feb 16, 2014 at 6:27 PM, Barry Dahlberg notifications@github.comwrote:

Looks fine to me [image: :+1:]

Reply to this email directly or view it on GitHubhttps://github.com/martin-magakian/Amazing-Cloud-Search/pull/12#issuecomment-35219914 .