[LyX/master] Try to fix bug #6505.
Richard Kimberly Heck
rikiheck at lyx.org
Sun Apr 26 02:12:50 UTC 2020
commit e4da31452273caeed7eabe7132f53201f44d5398
Author: Richard Kimberly Heck <rikiheck at lyx.org>
Date: Sat Apr 25 22:17:51 2020 -0400
Try to fix bug #6505.
Keep track of nested includes and just refuse to re-enter a file
we're already in the process of handling.
There's a question whether we should do this in updateBuffer and
validate, or whether we should do it separately. For now, this seems
to work.
---
src/Buffer.cpp | 50 ++++++++++++-
src/insets/InsetInclude.cpp | 165 +++++++++++++++++++++++--------------------
src/insets/InsetInclude.h | 4 +
3 files changed, 137 insertions(+), 82 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ebb302f..8b5fae1 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1095,6 +1095,10 @@ bool Buffer::readDocument(Lexer & lex)
d->old_position = params().origin;
else
d->old_position = filePath();
+
+ if (!parent())
+ clearIncludeList();
+
bool const res = text().read(lex, errorList, d->inset);
d->old_position.clear();
@@ -2400,6 +2404,9 @@ void Buffer::validate(LaTeXFeatures & features) const
if (!features.runparams().is_child)
params().validate(features);
+ if (!parent())
+ clearIncludeList();
+
for (Paragraph const & p : paragraphs())
p.validate(features);
@@ -2612,6 +2619,9 @@ void Buffer::reloadBibInfoCache(bool const force) const
void Buffer::collectBibKeys(FileNameList & checkedFiles) const
{
+ if (!parent())
+ clearIncludeList();
+
for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
it->collectBibKeys(it, checkedFiles);
if (it->lyxCode() == BIBITEM_CODE) {
@@ -3474,8 +3484,37 @@ bool Buffer::isReadonly() const
void Buffer::setParent(Buffer const * buffer)
{
- // Avoids recursive include.
- d->setParent(buffer == this ? nullptr : buffer);
+ // We need to do some work here to avoid recursive parent structures.
+ // This is the easy case.
+ if (buffer == this) {
+ LYXERR0("Ignoring attempt to set self as parent in\n" << fileName());
+ return;
+ }
+ // Now we check parents going upward, to make sure that IF we set the
+ // parent as requested, we would not generate a recursive include.
+ set<Buffer const *> sb;
+ Buffer const * b = buffer;
+ bool found_recursion = false;
+ while (b) {
+ if (sb.find(b) != sb.end()) {
+ found_recursion = true;
+ break;
+ }
+ sb.insert(b);
+ b = b->parent();
+ }
+
+ if (found_recursion) {
+ LYXERR0("Ignoring attempt to set parent of\n" <<
+ fileName() <<
+ "\nto " <<
+ buffer->fileName() <<
+ "\nbecause that would create a recursive inclusion.");
+ return;
+ }
+
+ // We should be safe now.
+ d->setParent(buffer);
updateMacros();
}
@@ -3496,8 +3535,6 @@ ListOfBuffers Buffer::allRelatives() const
Buffer const * Buffer::masterBuffer() const
{
- // FIXME Should be make sure we are not in some kind
- // of recursive include? A -> B -> A will crash this.
Buffer const * const pbuf = d->parent();
if (!pbuf)
return this;
@@ -4950,6 +4987,8 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
// do the real work
ParIterator parit = cbuf.par_iterator_begin();
+ if (scope == UpdateMaster)
+ clearIncludeList();
updateBuffer(parit, utype);
// If this document has siblings, then update the TocBackend later. The
@@ -4992,6 +5031,7 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
}
d->cite_labels_valid_ = true;
/// FIXME: Perf
+ clearIncludeList();
cbuf.tocBackend().update(true, utype);
if (scope == UpdateMaster)
cbuf.structureChanged();
@@ -5222,6 +5262,7 @@ void Buffer::Impl::setLabel(ParIterator & it, UpdateType utype) const
void Buffer::updateBuffer(ParIterator & parit, UpdateType utype, bool const deleted) const
{
+ pushIncludedBuffer(this);
// LASSERT: Is it safe to continue here, or should we just return?
LASSERT(parit.pit() == 0, /**/);
@@ -5277,6 +5318,7 @@ void Buffer::updateBuffer(ParIterator & parit, UpdateType utype, bool const dele
// set change indicator for the inset (or the cell that the iterator
// points to, if applicable).
parit.text()->inset().isChanged(changed);
+ popIncludedBuffer();
}
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index b0d2be4..be823a7 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -189,7 +189,8 @@ char_type replaceCommaInBraces(docstring & params)
InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams const & p)
: InsetCommand(buf, p), include_label(uniqueID()),
preview_(make_unique<RenderMonitoredPreview>(this)), failedtoload_(false),
- set_label_(false), label_(nullptr), child_buffer_(nullptr), file_exist_(false)
+ set_label_(false), label_(nullptr), child_buffer_(nullptr), file_exist_(false),
+ recursion_error_(false)
{
preview_->connect([=](){ fileChanged(); });
@@ -205,7 +206,7 @@ InsetInclude::InsetInclude(InsetInclude const & other)
: InsetCommand(other), include_label(other.include_label),
preview_(make_unique<RenderMonitoredPreview>(this)), failedtoload_(false),
set_label_(false), label_(nullptr), child_buffer_(nullptr),
- file_exist_(other.file_exist_)
+ file_exist_(other.file_exist_),recursion_error_(other.recursion_error_)
{
preview_->connect([=](){ fileChanged(); });
@@ -379,6 +380,7 @@ void InsetInclude::setParams(InsetCommandParams const & p)
// reset in order to allow loading new file
failedtoload_ = false;
+ recursion_error_ = false;
InsetCommand::setParams(p);
set_label_ = false;
@@ -446,12 +448,7 @@ docstring InsetInclude::screenLabel() const
Buffer * InsetInclude::getChildBuffer() const
{
- Buffer * childBuffer = loadIfNeeded();
-
- // FIXME RECURSIVE INCLUDE
- // This isn't sufficient, as the inclusion could be downstream.
- // But it'll have to do for now.
- return (childBuffer == &buffer()) ? nullptr : childBuffer;
+ return loadIfNeeded();
}
@@ -482,6 +479,9 @@ Buffer * InsetInclude::loadIfNeeded() const
return nullptr;
Buffer * child = theBufferList().getBuffer(included_file);
+ if (checkForRecursiveInclude(child))
+ return child;
+
if (!child) {
// the readonly flag can/will be wrong, not anymore I think.
if (!included_file.exists()) {
@@ -494,6 +494,7 @@ Buffer * InsetInclude::loadIfNeeded() const
// Buffer creation is not possible.
return nullptr;
+ buffer().pushIncludedBuffer(child);
// Set parent before loading, such that macros can be tracked
child->setParent(&buffer());
@@ -502,9 +503,11 @@ Buffer * InsetInclude::loadIfNeeded() const
child->setParent(nullptr);
//close the buffer we just opened
theBufferList().release(child);
+ buffer().popIncludedBuffer();
return nullptr;
}
+ buffer().popIncludedBuffer();
if (!child->errorList("Parse").empty()) {
// FIXME: Do something.
}
@@ -527,6 +530,26 @@ Buffer * InsetInclude::loadIfNeeded() const
}
+bool InsetInclude::checkForRecursiveInclude(
+ Buffer const * cbuf, bool silent) const
+{
+ if (recursion_error_)
+ return true;
+
+ if (!buffer().isBufferIncluded(cbuf))
+ return false;
+
+ if (!silent) {
+ docstring const msg = _("The file\n%1$s\n has attempted to include itself.\n"
+ "The document set will not work properly until this is fixed!");
+ frontend::Alert::warning(_("Recursive Include"),
+ bformat(msg, from_utf8(cbuf->fileName().absFileName())));
+ }
+ recursion_error_ = true;
+ return true;
+}
+
+
void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
{
string incfile = ltrim(to_utf8(params()["filename"]));
@@ -551,20 +574,6 @@ void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
}
FileName const included_file = includedFileName(buffer(), params());
-
- // Check we're not trying to include ourselves.
- // FIXME RECURSIVE INCLUDE
- // This isn't sufficient, as the inclusion could be downstream.
- // But it'll have to do for now.
- if (isInputOrInclude(params()) &&
- buffer().absFileName() == included_file.absFileName())
- {
- Alert::error(_("Recursive input"),
- bformat(_("Attempted to include file %1$s in itself! "
- "Ignoring inclusion."), from_utf8(incfile)));
- return;
- }
-
Buffer const * const masterBuffer = buffer().masterBuffer();
// if incfile is relative, make it relative to the master
@@ -803,6 +812,9 @@ void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
return;
}
+ if (recursion_error_)
+ return;
+
if (!runparams.silent) {
if (tmp->params().baseClass() != masterBuffer->params().baseClass()) {
// FIXME UNICODE
@@ -966,19 +978,13 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
// In the other cases, we will generate the HTML and include it.
- // Check we're not trying to include ourselves.
- // FIXME RECURSIVE INCLUDE
- if (buffer().absFileName() == included_file.absFileName()) {
- Alert::error(_("Recursive input"),
- bformat(_("Attempted to include file %1$s in itself! "
- "Ignoring inclusion."), ltrim(params()["filename"])));
- return docstring();
- }
-
Buffer const * const ibuf = loadIfNeeded();
if (!ibuf)
return docstring();
+ if (recursion_error_)
+ return docstring();
+
// are we generating only some paragraphs, or all of them?
bool const all_pars = !rp.dryrun ||
(rp.par_begin == 0 &&
@@ -995,6 +1001,7 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
<< from_utf8(included_file.absFileName())
<< XHTMLStream::ESCAPE_NONE
<< " -->";
+
return docstring();
}
@@ -1029,6 +1036,10 @@ int InsetInclude::plaintext(odocstringstream & os,
os << str;
return str.size();
}
+
+ if (recursion_error_)
+ return 0;
+
writePlaintextFile(*ibuf, os, op);
return 0;
}
@@ -1043,18 +1054,6 @@ int InsetInclude::docbook(odocstream & os, OutputParams const & runparams) const
return 0;
string const included_file = includedFileName(buffer(), params()).absFileName();
-
- // Check we're not trying to include ourselves.
- // FIXME RECURSIVE INCLUDE
- // This isn't sufficient, as the inclusion could be downstream.
- // But it'll have to do for now.
- if (buffer().absFileName() == included_file) {
- Alert::error(_("Recursive input"),
- bformat(_("Attempted to include file %1$s in itself! "
- "Ignoring inclusion."), from_utf8(incfile)));
- return 0;
- }
-
string exppath = incfile;
if (!runparams.export_folder.empty()) {
exppath = makeAbsPath(exppath, runparams.export_folder).realPath();
@@ -1067,6 +1066,9 @@ int InsetInclude::docbook(odocstream & os, OutputParams const & runparams) const
Buffer * tmp = loadIfNeeded();
if (tmp) {
+ if (recursion_error_)
+ return 0;
+
string const mangled = writefile.mangledFileName();
writefile = makeAbsPath(mangled,
buffer().masterBuffer()->temppath());
@@ -1136,40 +1138,41 @@ void InsetInclude::validate(LaTeXFeatures & features) const
// Load the file in the include if it needs
// to be loaded:
Buffer * const tmp = loadIfNeeded();
- if (tmp) {
- // the file is loaded
- // make sure the buffer isn't us
- // FIXME RECURSIVE INCLUDES
- // This is not sufficient, as recursive includes could be
- // more than a file away. But it will do for now.
- if (tmp && tmp != &buffer()) {
- // We must temporarily change features.buffer,
- // otherwise it would always be the master buffer,
- // and nested includes would not work.
- features.setBuffer(*tmp);
- // Maybe this is already a child
- bool const is_child =
- features.runparams().is_child;
- features.runparams().is_child = true;
- tmp->validate(features);
- features.runparams().is_child = is_child;
- features.setBuffer(buffer());
- }
- }
+ if (!tmp)
+ return;
+
+ // the file is loaded
+ if (checkForRecursiveInclude(tmp))
+ return;
+ buffer().pushIncludedBuffer(tmp);
+
+ // We must temporarily change features.buffer,
+ // otherwise it would always be the master buffer,
+ // and nested includes would not work.
+ features.setBuffer(*tmp);
+ // Maybe this is already a child
+ bool const is_child =
+ features.runparams().is_child;
+ features.runparams().is_child = true;
+ tmp->validate(features);
+ features.runparams().is_child = is_child;
+ features.setBuffer(buffer());
+
+ buffer().popIncludedBuffer();
}
void InsetInclude::collectBibKeys(InsetIterator const & /*di*/, FileNameList & checkedFiles) const
{
- Buffer * child = loadIfNeeded();
- if (!child)
+ Buffer * ibuf = loadIfNeeded();
+ if (!ibuf)
return;
- // FIXME RECURSIVE INCLUDE
- // This isn't sufficient, as the inclusion could be downstream.
- // But it'll have to do for now.
- if (child == &buffer())
+
+ if (checkForRecursiveInclude(ibuf))
return;
- child->collectBibKeys(checkedFiles);
+ buffer().pushIncludedBuffer(ibuf);
+ ibuf->collectBibKeys(checkedFiles);
+ buffer().popIncludedBuffer();
}
@@ -1189,7 +1192,7 @@ void InsetInclude::metrics(MetricsInfo & mi, Dimension & dim) const
} else {
if (!set_label_) {
set_label_ = true;
- button_.update(screenLabel(), true, false, !file_exist_);
+ button_.update(screenLabel(), true, false, !file_exist_ || recursion_error_);
}
button_.metrics(mi, dim);
}
@@ -1338,15 +1341,19 @@ void InsetInclude::addToToc(DocIterator const & cpit, bool output_active,
if (!childbuffer)
return;
+ if (checkForRecursiveInclude(childbuffer))
+ return;
+ buffer().pushIncludedBuffer(childbuffer);
// Update the child's tocBackend. The outliner uses the master's, but
// the navigation menu uses the child's.
childbuffer->tocBackend().update(output_active, utype);
// Include Tocs from children
childbuffer->inset().addToToc(DocIterator(), output_active, utype,
backend);
- //Copy missing outliner names (though the user has been warned against
- //having different document class and module selection between master
- //and child).
+ buffer().popIncludedBuffer();
+ // Copy missing outliner names (though the user has been warned against
+ // having different document class and module selection between master
+ // and child).
for (auto const & name
: childbuffer->params().documentClass().outlinerNames())
backend.addName(name.first, translateIfPossible(name.second));
@@ -1379,14 +1386,16 @@ void InsetInclude::updateCommand()
void InsetInclude::updateBuffer(ParIterator const & it, UpdateType utype, bool const deleted)
{
file_exist_ = includedFileExist();
-
- button_.update(screenLabel(), true, false, !file_exist_);
-
Buffer const * const childbuffer = getChildBuffer();
if (childbuffer) {
- childbuffer->updateBuffer(Buffer::UpdateChildOnly, utype);
+ if (!checkForRecursiveInclude(childbuffer))
+ childbuffer->updateBuffer(Buffer::UpdateChildOnly, utype);
+ button_.update(screenLabel(), true, false, recursion_error_);
return;
}
+
+ button_.update(screenLabel(), true, false, !file_exist_);
+
if (!isListings(params()))
return;
diff --git a/src/insets/InsetInclude.h b/src/insets/InsetInclude.h
index 6ba8b63..6636583 100644
--- a/src/insets/InsetInclude.h
+++ b/src/insets/InsetInclude.h
@@ -139,6 +139,9 @@ private:
bool isChildIncluded() const;
/// check whether the included file exist
bool includedFileExist() const;
+ /// \return True if there is a recursive include
+ /// Also issues appropriate warning, etc
+ bool checkForRecursiveInclude(Buffer const * cbuf, bool silent = false) const;
/// \name Private functions inherited from Inset class
//@{
@@ -173,6 +176,7 @@ private:
InsetLabel * label_;
mutable Buffer * child_buffer_;
mutable bool file_exist_;
+ mutable bool recursion_error_;
};
More information about the lyx-cvs
mailing list