[PR] Move HTML rendering from Message::Node::rendered () into Handler::HTML [spamassassin]

fkoyer opened a new pull request, #39:
URL: https://github.com/apache/spamassassin/pull/39

   Rendering an HTML part is the HTML handler's job, but handle_html() only called $node->rendered() to warm a cache and then read the results back. The parse now happens in the handler; rendered() renders text/plain and otherwise returns whatever the part's handler published with set_rendered().
   
   Mail::SpamAssassin::HTML is untouched.
   
   Note this makes the HTML handler load-bearing: with it not loaded, no HTML is parsed at all -- no rendered body text, no html_results for the html_* eval rules, and no URIs from href/src/action.  Previously only the URI harvest was lost.  v403.pre loads it by default (under the feature_handlers guard), so an install whose lib/ and rules/ come from the same version is unaffected; a site running this code against an older v403.pre, or one that has commented the loadhandler out, is not.  The existing warn in Message::apply_handlers() for a text/html part with no registered handler covers this; it now says what is actually lost, and has moved inside the part-walking loop, which reuses the per-node handler lookup already done there instead of a separate find_parts() scan.  Two cases it used 
 to miss now warn: a message whose only text/html part is a synthetic child produced by another handler (ICS, Archive), and a config with no handlers registered at all -- the latter returned early, b
 efore the check.
   
   Node.pm:
    - new decode_and_normalize(): decode() plus the charset decision, factored out of the two branches of rendered() that each had their own copy. Returns ($text, $character_semantics) so a caller feeding a parser that must be told whether it is getting bytes or characters can do so.
    - set_rendered() takes optional $invisible and $visible arguments, for a renderer that can tell hidden text from shown text.  Both default to the old behaviour, so existing callers are unaffected.
    - rendered() no longer parses HTML; its type gate is text/plain only.
   
   Handler/SVG.pm now publishes the text it extracts with set_rendered(), under the part's own image/svg+xml type.  Previously an SVG part was rendered only as a side effect of rendered()'s HTML branch, which stamped it text/html -- so an SVG arriving as an attachment (the common case) was then dropped from the body by the text/* attachment skip in get_body_text_array_common().
   
   This is a behaviour change, not just code motion: SVG text now reaches body rules where it did not before, the same way OCR'd image and PDF text already do.  On t/data/nice/handler_svg the body gains "SVGSENTINEL Please view documents on docusign secure link".
   
   Bayes asks for the MIME-part handlers explicitly on the learn path (get_body_from_msg).  It already got them, but only incidentally: it calls Message::extract_message_metadata, which does not run handlers, and then _get_msgdata_from_permsgstatus -> get_uri_list -> get_uri_detail_list, which does.  The handler-extracted text Bayes learns therefore hung on a call made for its URIs.  apply_handlers() is idempotent, so this is a no-op at runtime; it just stops the token stream depending on that.
   
   One smaller behaviour change: text/plain parts now emit the "normalize_charset is off" debug line, which only the HTML branch used to log.  Debug output only.
   
   Also:
    - Pyzor::Digest::Pieces read {rendered} directly, which only worked if something else had already rendered the part; call rendered().
    - Message::get_body_text_array_common no longer pushes an undef into {metadata}{html_all} when a handler publishes text/html without html_results.
    - t/scan_text_attach.t builds a PerMsgStatus and calls apply_handlers(), as a real scan does; it previously relied on rendered() lazily parsing HTML with no handler having run.
   
   Verified behaviour-neutral for HTML: rendered/visible/invisible text, utf8 flags and the html_results stats are byte-identical across 70 rendered parts from 23 fixtures, with normalize_charset both on and off.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]