Review Request: sieve variables extension (dev/cassell/for-review-sieve-variables-rev1)
James Cassell
fedoraproject at cyberpear.com
Mon Jan 12 00:19:31 EST 2015
Hello,
I believe I have a more or less complete implementation of RFC 5229,
Sieve: Variables Extension.
tl;dr: I'd like help making sure that the sanity checks currently
performed at script compile time are also performed at runtime for
the actions (or tests) that require it.
At this point, I have cleaned up the patches, and I plan to drop the
ones that haven't been cleaned up since they were mostly for
debugging. I'm open to any feedback if anyone would like the commits
reworked in a particular way.
I plan to add some more CUnit tests to get some ongoing automated
testing to make sure nothing breaks going forward, though I'm (as
always) short on time.
One problem that I have thought of is that checks performed in the
sieve parser (sieve/sieve.y) will possibly need to be performed at
runtime if the variables extension is enabled. These checks are in
verify_utf8, verify_regex, verify_envelope, verify_addrheader,
verify_header, verify_mailbox, and verify_address, in addition to any
one-off checks in any individual rules. It appears that this will
apply mainly to actions, and possibly to the regex match-type. It
might also apply to what happens if an address or envelope test is
applied to a non-address containing header, or if a test becomes
malformed. In such a case, should we have the test return false, or
should we abort the script and fallback to the implicit keep? The
thing I hate about runtime errors is that there is no notification to
the script author that they happened.
Updates from last time:
On Tue, Nov 11, 2014, at 11:12 AM, James Cassell wrote:
[...]
> What works:
> - setting variables using the SET action
> - match variables are set when the :matches match-type is used (${0},
> ${1}, etc)
> - did this by overloading the comprock, which had been null for
> :matches test
> - should I do this differently?
> - most of the places that you would normally use a string, you can also
> add variables to your strings
>
This all still works.
> What doesn't work:
> - modifiers to the set action
Done
> - the STRING test
Done
> - :regex match-type
> - Ken, I might need your help on this one
>
I don't think I'll get to this one and I don't consider it crucial
since its not an official RFC.
> What I don't know how to do:
> - evaluate for UTF-8 support
I still have no idea what I should be looking for here.
[...]
> What I'd like reviewed:
> - the code
> - the above stuff that I don't know
> - security issues due to VARIABLES interaction with sieve actions
> - anything else you can think of
>
Still applies
> Also, I believe the first five patches are acceptable for master. (I
> won't push them, though, until I've given people a chance to look them
> over if they want.)
>
At this point, I'll likely just push them when merging variables to
master.
I look forward to any feedback!
Thanks!
V/r,
James Cassell
The following changes since commit e1ff81794b435586d640fa41e16a285f8bccdeb9:
[doc] clarify how to write CUnit tests (2015-01-02 21:21:53 -0500)
are available in the git repository at:
git://git.cyrusimap.org/cyrus-imapd.git dev/cassell/for-review-sieve-variables-rev1
for you to fetch changes up to 099c7abf2bf98ea8ce9fa44c6f93585587e6734d:
[sieve] grammar.c: check attempted use of namespaced variables (2015-01-09 17:32:04 -0500)
----------------------------------------------------------------
James Cassell (50):
[sieve] bc_eval.c: allow {add,set,remove}flag actions with reject
[sieve] change imap4flags variable names to support VARIABLES extension
[sieve] use central variable_list_t to hold action flag lists
[sieve] bc_eval.c: do all imap4flags processing in bc_eval.c
[sieve] remove do_{{add,set,remove}flag,{un,}mark}() functions
[sieve] sieve.y: define tokens for VARIABLES extension
[sieve] sieve-lex.l: teach lexer VARIABLES tokens
[sieve] add VARIABLES reference
[sieve] enable awareness of the variables extension
[sieve] add parser support for string test
[sieve] add parser support for SET action
[sieve] bytecode.h: add codepoints required by variables base spec
[sieve] bc_generate.c: variables base spec support
[sieve] bc_dump.c: variables base spec support
[sieve] bc_emit.c: variables base spec support
sieve test script
[sieve] sieved.c: variables base spec support
[sieve] store require'd extensions in the bytecode
[sieve] implement parse_string() to resolve variables in strings
[sieve] special variables for storing match vars and parsed strings
[sieve] implement variables SET action
debug printf
[sieve] bc_eval.c: pass variables to eval_bc_test()
[sieve] add support for match variables to :matches match-type
[sieve] comparator.c: *_matches(): only use given void *rock if non-NULL
[sieve] grammar.c: parse_string(): Allow parsing of match variables
[sieve] bc_eval.c: parse strings for variables when reading bytecode
[sieve] bc_eval.c: attempt proper variables support in imap4flags
break line to avoid horiz scrolling
BC_STRING maps to BC_HASFLAG
test script changes
temp
[sieve] bc_eval.c: implement variables STRING test
update for debugging
[sieve] sieve.y: support variables in HASFLAG test
[sieve] bc_eval.c: support non-existent variable in hasflag test
[sieve] variables.[ch]: template for SET action modifiers
[sieve] variables.c: allocate buffer for resultant modified string
[sieve] bc_eval.c: body test should not set match variables
[sieve] variables.c: implement and test set action modifiers
[cunit] sieve: :quotewildcard with :encodeurl in variable_modifiers
[sieve] bc_eval.c: apply specified variable modifiers in set action
[sieve] support variables extension in setflag action
[sieve] parser support for {add,remove}flag actions with variables
[sieve] support variables extension in {add,remove}flag actions
[sieve] bc_eval.c: plan support for :count with variables in tests
[sieve] support :count with variables in hasflag and string tests
[sieve] bc_eval.c: break from loop after calculating :count
[sieve] flags.[ch]: properly use EXPORTED
[sieve] grammar.c: check attempted use of namespaced variables
Makefile.am | 4 +
cunit/sieve.testc | 79 +++-
doc/specs.html | 2 +
lib/imapoptions | 2 +-
sieve/README | 3 +
sieve/bc_dump.c | 59 ++-
sieve/bc_emit.c | 84 +++-
sieve/bc_eval.c | 517 ++++++++++++++++++---
sieve/bc_generate.c | 105 ++++-
sieve/bytecode.h | 34 +-
sieve/comparator.c | 96 +++-
sieve/flags.c | 4 +
sieve/flags.h | 2 +-
sieve/grammar.c | 167 +++++++
sieve/grammar.h | 18 +
sieve/message.c | 84 ----
sieve/script.c | 67 +--
sieve/script.h | 1 +
sieve/sieve-lex.l | 16 +-
sieve/sieve.y | 309 ++++++++++--
sieve/sieve_interface.h | 1 +
sieve/sieved.c | 63 ++-
.../actionExtensions/uberExtensionActionScript.s | 76 +--
.../tests/testExtension/uberExtensionTestScript.s | 59 ++-
sieve/tree.c | 9 +-
sieve/tree.h | 12 +-
sieve/variables.c | 197 ++++++++
sieve/variables.h | 13 +
sieve/varlist.h | 2 +
29 files changed, 1724 insertions(+), 361 deletions(-)
create mode 100644 sieve/grammar.c
create mode 100644 sieve/grammar.h
create mode 100644 sieve/variables.c
create mode 100644 sieve/variables.h
More information about the Cyrus-devel
mailing list