TrueCloudLab/frostfs-sdk-go

Fork 20

Fix policy encoding to string #104

Merged

fyrchik merged 2 commits from alexvanin/frostfs-sdk-go:fix/policy-encode-characters into master

2023-07-07 09:33:02 +00:00

alexvanin commented

2023-07-05 12:19:25 +00:00

Owner

Open questions:

Where special symbols are allowed besides filter keys and values (selector and filter names?)
What special symbols are allowed for filter keys and values besides whitespace and -

Open questions: 1. Where special symbols are allowed besides filter keys and values (selector and filter names?) 2. What special symbols are allowed for filter keys and values besides whitespace and `-`

alexvanin force-pushed fix/policy-encode-characters from edf5d9e9db to a4fb25bc72

2023-07-05 12:19:53 +00:00

Compare

alexvanin requested review from storage-core-committers 2023-07-05 12:19:54 +00:00

alexvanin requested review from storage-core-developers 2023-07-05 12:19:54 +00:00

alexvanin force-pushed fix/policy-encode-characters from a4fb25bc72 to 92f6bb9736

2023-07-05 12:24:38 +00:00

Compare

fyrchik commented

2023-07-05 12:24:47 +00:00

Owner

According to the grammar, special symbols are only allowed in STRING, so nowhere besides filter keys and values. https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/netmap/parser/Query.g4#L42

Again, according to the grammar, \t etc is allowed https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/netmap/parser/QueryLexer.g4#L32
BTW, one of the solutions is to marshal string in JSON instead of custom escaping function.

According to the grammar, special symbols are only allowed in `STRING`, so nowhere besides filter keys and values. https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/netmap/parser/Query.g4#L42 Again, according to the grammar, `\t` etc is allowed https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/netmap/parser/QueryLexer.g4#L32 BTW, one of the solutions is to marshal string in JSON instead of custom escaping function.

dstepanov-yadro approved these changes 2023-07-05 13:39:13 +00:00

fyrchik commented

2023-07-05 14:22:45 +00:00

Owner

Linter warning was fixed in #105.

alexvanin commented

2023-07-05 14:29:57 +00:00

Author

Owner

@fyrchik added \t

BTW, one of the solutions is to marshal string in JSON instead of custom escaping function.

JSON marshaler returns error which we can't ignore, a bit uncomfortable.

Also with JSON marshaler wraps all keys and values with ", so it may look a bit weird.

"Rating" GE "5"

@fyrchik added \t > BTW, one of the solutions is to marshal string in JSON instead of custom escaping function. JSON marshaler returns error which we can't ignore, a bit uncomfortable. Also with JSON marshaler wraps all keys and values with `"`, so it may look a bit weird. ``` "Rating" GE "5" ```

alexvanin force-pushed fix/policy-encode-characters from 92f6bb9736 to 7ad0ce3c34

2023-07-05 14:30:29 +00:00

Compare

acid-ant approved these changes 2023-07-06 08:08:20 +00:00

dkirillov approved these changes 2023-07-06 08:31:03 +00:00

alexvanin commented

2023-07-07 08:57:44 +00:00

Author

Owner

Let's merge it for now and I'll come back later with new issues / PRs if there will be more characters to escape in policies. Right now white space and dash are used for UN-LOCODE filters.

Let's merge it for now and I'll come back later with new issues / PRs if there will be more characters to escape in policies. Right now white space and dash are used for `UN-LOCODE` filters.

fyrchik approved these changes 2023-07-07 09:32:55 +00:00