79 lines
3.2 KiB
Markdown
79 lines
3.2 KiB
Markdown
The remove plugin cannot remove [[todo/transient_pages]].
|
|
|
|
> this turns out to be harder than
|
|
> I'd hoped, because I don't want to introduce a vulnerability in the
|
|
> non-regular-file detection, so I'd rather defer that. --[[smcv]]
|
|
|
|
This is particularly a problem for tag pages, and autoindex
|
|
created pages. So both plugins default to not creating transient
|
|
pages, until this is fixed. --[[Joey]]
|
|
|
|
> I'll try to work out which of the checks are required for security
|
|
> and which are just nice-to-have, but I'd appreciate any pointers
|
|
> you could give. --[[smcv]]
|
|
|
|
>> I assume by "non-regular file", you are referring to the check
|
|
>> in remove that the file "Must exist on disk, and be a regular file" ?
|
|
>> --[[Joey]]
|
|
|
|
>>> Yes. It's not entirely clear to me why that's there... --s
|
|
|
|
>>>> Yeah, 2461ce0de6231bfeea4d98c86806cdbb85683297 doesn't really
|
|
>>>> say, and I tend to assume that when I've written paranoid code
|
|
>>>> it's there for a reason. I think that here the concern was that
|
|
>>>> the file might be in some underlay that the user should not be able
|
|
>>>> to affect by web edits. The `-f` check seems rather redundant,
|
|
>>>> surely if it's in `%pagesources` ikiwiki has already verified it's
|
|
>>>> safe. --[[Joey]]
|
|
|
|
----
|
|
|
|
[[!template id=gitbranch branch=smcv/ready/transient-rm author="[[Simon McVittie|smcv]]"]]
|
|
|
|
Here's a branch. It special-cases the `$transientdir`, but in such a way
|
|
that the special case could easily be extended to other locations where
|
|
deletion should be allowed.
|
|
|
|
It also changes `IkiWiki::prune()` to optionally stop pruning empty
|
|
parent directories at the point where you'd expect it to (for instance,
|
|
previously it would remove the `$transientdir` itself, if it turns out
|
|
to be empty), and updates callers.
|
|
|
|
The new `prune` API looks like this:
|
|
|
|
IkiWiki::prune("$config{srcdir}/$file", $config{srcdir});
|
|
|
|
with the second argument optional. I wonder whether it ought to look
|
|
more like `writefile`:
|
|
|
|
IkiWiki::prune($config{srcdir}, $file);
|
|
|
|
although that would be either an incompatible change to internal API
|
|
(forcing all callers to update to 2-argument), or being a bit
|
|
inconsistent between the one-and two-argument forms. Thoughts?
|
|
|
|
--[[smcv]]
|
|
|
|
> I've applied the branch as-is, so this bug is [[done]].
|
|
> `prune` is not an exported API so changing it would be ok..
|
|
> I think required 2-argument would be better, but have not checked
|
|
> all the call sites to see if the `$file` is available split out
|
|
> as that would need. --[[Joey]]
|
|
|
|
[[!template id=gitbranch branch=smcv/ready/prune author="[[Simon McVittie|smcv]]"]]
|
|
|
|
>> Try this, then? I had to make some changes to `attachment`
|
|
>> to make the split versions available. I suggest reviewing
|
|
>> patch-by-patch.
|
|
|
|
>>> Branch updated; I'd missed a use of prune in ikiwiki.in itself.
|
|
>>> Unfortunately, this means it does still need to support the
|
|
>>> "undefined top directory" case: there isn't an obvious top
|
|
>>> directory for wrappers. --[[smcv]]
|
|
|
|
>> I also tried to fix a related bug which I found while testing it:
|
|
>> the special case for renaming held attachments didn't seem to work.
|
|
>> (`smcv/wip/rename-held`.) Unfortunately, it seems that with that
|
|
>> change, the held attachment is committed to the `srcdir` when you
|
|
>> rename it, which doesn't seem to be the intention either? --[[smcv]]
|