From ef3157e0ec304081de3d28e9f222638a0de3ae90 Mon Sep 17 00:00:00 2001 From: Jim Derry Date: Thu, 1 Jul 2021 15:36:27 -0400 Subject: [PATCH] Fix issues with user-specified settings changing User-specified settings were being fiddled with by tidy internally. User settings set by the user should always be able to be read back by the user, but `AdjustConfig()` would change them. This change contributes toward fixing the situation by using `AdjustConfig()` at the only point that it's needed, as well by NOT automatically reverting to the snapshot after outputting a buffer (which should be stateless, but caused PHP to break because it tried to save buffer more than once, so subsequent calls would use non-fiddle settings.). --- include/tidy.h | 8 ++++---- src/config.c | 22 +++++++++++++++++----- src/config.h | 14 ++++++++++++++ src/tidylib.c | 8 +++++--- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/include/tidy.h b/include/tidy.h index da80857..35c3b21 100644 --- a/include/tidy.h +++ b/include/tidy.h @@ -548,10 +548,10 @@ TIDY_EXPORT int TIDY_CALL tidySetOutCharEncoding(TidyDoc tdoc, /**< The ** @note In general, you should expect that options you set should stay set. ** This isn't always the case, though, because Tidy will adjust options ** for internal use during the lexing, parsing, cleaning, and printing - ** phases, but will restore them after the printing process. If you - ** require access to user configuration values at any time between the - ** tidyParseXXX() process and the tidySaveXXX() process, make sure to - ** keep your own copy. + ** phases. If you require access to user configuration values at any + ** time after the tidyParseXXX() process, make sure to keep your own + ** copy, or use tidyOptResetToSnapshot() when you no longer need to + ** use any other tidy functions. ** @{ ******************************************************************************/ diff --git a/src/config.c b/src/config.c index bae56b8..4a132f5 100644 --- a/src/config.c +++ b/src/config.c @@ -304,7 +304,6 @@ static const struct { /* forward declarations */ -static void AdjustConfig( TidyDocImpl* doc ); static Bool GetPickListValue( ctmbstr value, PickListItems* pickList, uint *result ); @@ -713,7 +712,11 @@ void TY_(TakeConfigSnapshot)( TidyDocImpl* doc ) const TidyOptionValue* value = &doc->config.value[ 0 ]; TidyOptionValue* snap = &doc->config.snapshot[ 0 ]; - AdjustConfig( doc ); /* Make sure it's consistent */ + /* @jsd: do NOT mess with user-specified settings until + * absolutely necessary, and ensure that we can + * can restore them immediately after the need. + */ + // TY_(AdjustConfig)( doc ); /* Make sure it's consistent */ for ( ixVal=0; ixVal < N_TIDY_OPTIONS; ++option, ++ixVal ) { assert( ixVal == (uint) option->id ); @@ -762,7 +765,12 @@ void TY_(CopyConfig)( TidyDocImpl* docTo, TidyDocImpl* docFrom ) } if ( needReparseTagsDecls ) ReparseTagDecls( docTo, changedUserTags ); - AdjustConfig( docTo ); /* Make sure it's consistent */ + + /* @jsd: do NOT mess with user-specified settings until + * absolutely necessary, and ensure that we can + * can restore them immediately after the need. + */ + // TY_(AdjustConfig)( docTo ); /* Make sure it's consistent */ } } @@ -1074,7 +1082,11 @@ int TY_(ParseConfigFileEnc)( TidyDocImpl* doc, ctmbstr file, ctmbstr charenc ) if ( fname != (tmbstr) file ) TidyDocFree( doc, fname ); - AdjustConfig( doc ); + /* @jsd: do NOT mess with user-specified settings until + * absolutely necessary, and ensure that we can + * can restore them immediately after the need. + */ + // TY_(AdjustConfig)( docTo ); /* Make sure it's consistent */ /* any new config errors? If so, return warning status. */ return (doc->optionErrors > opterrs ? 1 : 0); @@ -1214,7 +1226,7 @@ Bool TY_(AdjustCharEncoding)( TidyDocImpl* doc, int encoding ) /* ensure that config is self consistent */ -static void AdjustConfig( TidyDocImpl* doc ) +void TY_(AdjustConfig)( TidyDocImpl* doc ) { if ( cfgBool(doc, TidyEncloseBlockText) ) TY_(SetOptionBool)( doc, TidyEncloseBodyText, yes ); diff --git a/src/config.h b/src/config.h index d89338d..a86b5c5 100644 --- a/src/config.h +++ b/src/config.h @@ -335,6 +335,20 @@ Bool TY_(ParseConfigValue)( TidyDocImpl* doc, TidyOptionId optId, ctmbstr optVa Bool TY_(AdjustCharEncoding)( TidyDocImpl* doc, int encoding ); +/** Ensure that the configuration options are self consistent. + ** THIS PROCESS IS DESTRUCTIVE TO THE USER STATE. It examines + ** certain user-specified options and changes other options + ** as a result. This means that documented API functions such + ** as tidyOptGetValue() won't return the user-set values after + ** this is used. As a result, *don't* just use this function + ** at every opportunity, but only where needed, which is ONLY + ** prior to parsing a stream, and again prior to saving a + ** stream (because we reset after parsing.) + ** @param doc The Tidy document to adjust. + */ +void TY_(AdjustConfig)( TidyDocImpl* doc ); + + /** Indicates whether or not the current configuration is completely default. ** @param doc The Tidy document. ** @returns The result. diff --git a/src/tidylib.c b/src/tidylib.c index de08d91..4485d1a 100644 --- a/src/tidylib.c +++ b/src/tidylib.c @@ -1462,8 +1462,9 @@ int TY_(DocParseStream)( TidyDocImpl* doc, StreamIn* in ) assert( doc->docIn == NULL ); doc->docIn = in; - TY_(ResetTags)(doc); /* reset table to html5 mode */ - TY_(TakeConfigSnapshot)( doc ); /* Save config state */ + TY_(ResetTags)(doc); /* Reset table to html5 mode */ + TY_(TakeConfigSnapshot)( doc ); /* Save config state */ + TY_(AdjustConfig)( doc ); /* Ensure config internal consistency */ TY_(FreeAnchors)( doc ); TY_(FreeNode)(doc, &doc->root); @@ -2283,7 +2284,8 @@ int tidyDocSaveStream( TidyDocImpl* doc, StreamOut* out ) doc->docOut = NULL; } - TY_(ResetConfigToSnapshot)( doc ); + /* @jsd: removing this should solve #673, and allow saving of the buffer multiple times. */ +// TY_(ResetConfigToSnapshot)( doc ); doc->pConfigChangeCallback = callback; return tidyDocStatus( doc );