Search strings with a forward slash followed by a numeric do not work via the path based search API

[adamralph]

Due to this string replacement:

• /messages/search/foo/bar ✔️ works
• /messages/search/foo/123 ❌ does not work!

I stepped through the code and it appears that this is some weird quirk of RavenDB. When searching for "foo\bar", RavenDB matches "foo/bar", but when searching for "foo\1", RavenDB does not match "foo/1". 😕

Note that there are four variants of the search API and only one of them does this replacement in the keyword. Something definitely seems off with this.

Ultimately, if you arbitrarily replace a character in a search string, then obviously no client will ever be able to search using that character. And to only do it for one variant of the search is even worse:

1. /messages/search?q=foo/123 - ✔️ works
2. /messages/search/foo/123 - ❌ does not work!
3. /endpoints/myendpoint/messages/search?q=foo/123 - ✔️ works
4. /endpoints/myendpoint/messages/search/foo/123 - ✔️ works

To fix the immediate problem, we can switch ServiceInsight to using the query string variant (1) which doesn't do the replacement, but still, it doesn't seem right to leave variant (2) in this broken state.

Previous description

The search API appears to support two formats: - `http://localhost:33333/api/messages/search/foo` (used by ServiceInsight) - `http://localhost:33333/api/messages/search?q=foo` However, strings containing forward slashes. For example, when searching for the string "foo/123", the correct, encoded URLs would be: - `http://localhost:33333/api/messages/search/foo%2f123` - `http://localhost:33333/api/messages/search?q=foo%2f123` But neither of these return any results. The only way to search for "foo/123" is to use the following un-encoded URL: - `http://localhost:33333/api/messages/search?q=foo/123` (There is no equivalent of the other URL format that works.) ServiceInsight correctly encodes the URL's. Until this is fixed, there is no way to search for strings which contain characters which need to be URL encoded from ServiceInsight.

[adamralph]

@Particular/servicecontrol-maintainers any thoughts on this one?

[adamralph]

It turns out that this is only a problem for search strings containing forward slashes, due this deliberate manipulation: https://github.com/Particular/ServiceControl/blob/10f82ebab164c62833fd3c71f9ead21ff346955e/src/ServiceControl/CompositeViews/Messages/GetMessagesByQuery.cs#L25

[WojcikMike]

@adamralph I looked at the code that it refers to: https://github.com/Particular/ServiceControl/blob/master/src/ServiceControl/CompositeViews/Messages/GetMessagesByQuery.cs#L15-L27

And run your sample URL's. Below is what I found:

• using search?q=foo/123 and search?q=foo%2f123 search using the same string which is foo/123 so those 2 calls should work in the same way
• using search/foo%2f123 falls into weird line: https://github.com/Particular/ServiceControl/blob/master/src/ServiceControl/CompositeViews/Messages/GetMessagesByQuery.cs#L25 That replaces / into \\. And it's becaouse this commit: https://github.com/Particular/ServiceControl/commit/05333da933ae7909e6b86659f7eeabb6e69178ae

@SzymonPobiega @gbiellem do you remember by any chance why this is done?

[adamralph]

@WojcikMike it looks like we crossed messages 😄

[WojcikMike]

@adamralph you posted just before me. Aparently replace was to fix an issue with MSMQ, but when I read it: https://github.com/Particular/ServiceControl/issues/438 I couldnt see how does it help :|

[gbiellem]

@adamralph @wojcikmike - wow that commit is from two years ago. Apologies but no I don;'t remember the details at all.

[adamralph]

Note that there are four variants of the search API and only one of them does this replacement in the keyword. Something definitely seems off with this.

Ultimately, if you arbitrarily replace a character in a search string, then obviously no client will ever be able to search using that character. And to only do it for one variant of the search is even worse:

1. /messages/search?q=foo/123 - ✔️ works
2. /messages/search/foo/123 - ❌ does not work!
3. /endpoints/myendpoint/messages/search?q=foo/123 - ✔️ works
4. /endpoints/myendpoint/messages/search/foo/123 - ✔️ works

To fix the immediate problem, we can switch ServiceInsight to using the query string variant (1) which doesn't do the replacement, but still, it doesn't seem right to leave variant (2) in this broken state.

[adamralph]

The plot thickens... it turns out that this is only a problem when the part after the forward slash is numeric. I.e.

• /messages/search/foo/bar ✔️ works
• /messages/search/foo/123 ❌ does not work!

I stepped through the code and it appears that this is some weird quirk of RavenDB. When searching for "foo\bar", RavenDB matches "foo/bar", but when searching for "foo\1", RavenDB does not match "foo/1". 😕

I suspect this why the replacement of the forward slash with a back slash made it through testing.

[adamralph]

I've updated the title and description of this issue accordingly.

[gbiellem]

Nice detective skills @adamralph

[adamralph]

We've fixed the immediate problem for SI by switching to the query string API.

I'll leave it to @Particular/servicecontrol-maintainers to decide what to do this issue, and #922.

[SzymonPobiega]

Not sure if there is anything here to do since SI is fine according to the latest comment. I propose to close this one.