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