Skip to content

check for undesirable quotes - #260

Merged
mbrt merged 1 commit into
mbrt:masterfrom
mefuller:check_quoted
Jun 20, 2022
Merged

mbrt merged 1 commit into
mbrt:masterfrom
mefuller:check_quoted

Conversation

@mefuller

@mefuller mefuller commented Jun 16, 2022 •

Copy link
Copy Markdown
Contributor

Fixes #66.

It's also my first attempt to meaningfully work on go/golang, so any guidance or pointers are very appreciated

Since processed strings are enclosed in outer double quotes, I test for a double quote contained anywhere as well as for enclosing single quotes, which would then be captured inside double quotes and result in a mismatch

Thanks

@mbrt mbrt left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! I left some comments to simplify things a bit. Hopefully it's helpful.

Comment thread internal/engine/filter/convert.go Outdated
Comment thread internal/engine/filter/convert.go Outdated
Comment thread internal/engine/filter/convert.go Outdated
Comment thread internal/engine/filter/convert.go Outdated
Comment thread internal/engine/filter/convert.go
@mefuller

Copy link
Copy Markdown
Contributor Author

@mbrt thank you for your help and patience.
I updated convert.go per your suggestions and will work on writing a test next

Comment thread internal/engine/filter/convert.go Outdated
Comment thread internal/engine/filter/convert.go
Comment thread internal/engine/filter/convert_test.go
Comment thread internal/engine/filter/convert.go
@mefuller

Copy link
Copy Markdown
Contributor Author

I think this is finally "correct" and I have squashed and reworded my commits appropriately.
Thank you very much again for working with me

@mbrt mbrt left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! We're almost there.

Comment thread internal/engine/filter/convert.go Outdated
@mbrt

mbrt commented Jun 20, 2022

Copy link
Copy Markdown
Owner

Thank you @mefuller! This is good to go.

@mbrt
mbrt merged commit 04dde7e into mbrt:master Jun 20, 2022
@mefuller

Copy link
Copy Markdown
Contributor Author

Thank you @mbrt

gpechenik pushed a commit to gpechenik/gmailctl that referenced this pull request Oct 4, 2026
Gmail's search syntax has no way to escape a double quote, so a `"`
inside a value can only be valid as a pair around the whole value,
e.g. subject: '"hello world"'. Since bc6bb81 such values are passed
through unchanged, which fixed the example originally reported in mbrt#66.
Any other quote still produced a malformed query without warning:

    subject: 'say "hi" now'       ->  subject:"say "hi" now"
    subject: '"unbalanced start'  ->  subject:""unbalanced start"
    from: 'foo"bar'               ->  from:foo"bar
    has: '"exact phrase" other'   ->  ""exact phrase" other"

The pass-through check also only looked at the first and last
characters, so '"foo" OR "bar"' was treated as a single phrase. Nested
in a larger query it rendered as `subject:"foo" OR "bar"`, where "bar"
matches anywhere in the message, not only in the subject.

Make quote() accept a quote only when the value is exactly one quoted
phrase (the new filter.IsQuoted), and fail otherwise, with a note on
how to fix the config: remove the quotes (they are added automatically
when needed), or write raw Gmail syntax with `isEscaped: true` or
`query`, which are not checked.

A similar check was added in mbrt#260 and reverted in mbrt#299, because it
rejected every quote, including values wrapped as a whole, which users
rely on to force an exact match (e.g. to: '"foo+bar"'). Those keep
working: the new check only rejects values that already generated
broken queries. Configs containing them will now fail to apply until
fixed, which is intended, but worth a release note.

`gmailctl test` also treated the quotes around a phrase as literal
text, so rules like subject: '"hello world"' or to: '"foo+bar"' failed
their own tests, even though Gmail matches them. The evaluators now
strip those quotes before matching, using the same IsQuoted check.

Close mbrt#66.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subject (and possibly other rules) with quotes

2 participants