Skip to content

Fix example plugin problems found by fuzzing: Test, ChoiceParams, ColourSpace, DepthConverter - #281

Open
garyo wants to merge 7 commits into
mainfrom
fix/examples-fuzzing
Open

garyo wants to merge 7 commits into
mainfrom
fix/examples-fuzzing

Conversation

@garyo

@garyo garyo commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fuzzing the plugins with a new command-line test host (not yet public) turned these up.

  • Test failed GetClipPreferences. The PropertyTestPlugin checks each action's handles against its own list of which may be null, and that list had GetClipPreferences requiring inArgs, which the specification passes as NULL.
  • ChoiceParams returned kOfxStatOK for clip preferences it never set. Its only preference is the output depth, which it sets only when the host supports multiple clip depths; otherwise it set nothing and still answered kOfxStatOK rather than kOfxStatReplyDefault.
  • ColourSpace:
    • It answered OK to actions it ignores. Its main entry started from kOfxStatOK, so GetFramesNeeded, Begin/EndSequenceRender, PurgeCaches, SyncPrivateData and the instance edit and change brackets all returned kOfxStatOK with nothing done.
    • Its choice parameters had no enum values on a host without colour management. The "[Unspecified]" and "[Same as input]" entries were marked for the Basic style only, so every entry was skipped, leaving three string-choice parameters with no values and no valid default. Both entries are now offered in every style, along with other minor fixes.
    • Its text overran small frames. drawText clipped at the top and right but not the bottom or left, and the text sits 100 pixels in, so on a frame under about 200 pixels high it wrote before the start of the image data.
    • It rendered tiles wrongly. Render copied the source with one memcpy of the whole frame, starting at the output's data pointer. The text is now placed relative to the output's region of definition.
  • DepthConverter set an output depth the host can't give it. It set the output clip's depth from its parameter whatever the host said, but that's disallowed on hosts that don't support multiple clip depths. It also answered kOfxStatOK when its parameter mapped to no depth and it had set nothing. It now keeps the host's preferences in both cases, with kOfxStatReplyDefault.

Assisted-by: Claude Code / Claude Opus 5.5

garyo and others added 7 commits September 24, 2026 11:34
… inArgs

The PropertyTestPlugin checks each action's handles against its own
list of which ones may be null, and that list had GetClipPreferences
requiring inArgs. The spec passes NULL inArgs to that action, so the
plugin answered kOfxStatErrBadHandle and a conforming host could not
finish setting up an instance. Every other action's entry matches the
spec.

Once past that check, the action answered kOfxStatOK having set
nothing, which the spec reserves for an action that changed at least
one out-arg. It now answers kOfxStatReplyDefault.

Found by the new test host.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
…ever set

Its only clip preference is the output depth, which it sets when the
host supports multiple clip depths. On any other host it set nothing
and still answered kOfxStatOK, which the spec reserves for an action
that changed at least one out-arg. It now answers
kOfxStatReplyDefault there, before querying the source clip for a
depth it has no use for.

Found by the new test host, which warns when a plugin answers
kOfxStatOK with nothing written.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Its main entry started from kOfxStatOK and kept it for any action it
did not handle, so GetFramesNeeded, Begin/EndSequenceRender,
PurgeCaches, SyncPrivateData and the instance edit and change
brackets were all answered kOfxStatOK with nothing done. The spec says
a plugin returns kOfxStatReplyDefault for an action it does not trap,
and for GetFramesNeeded kOfxStatOK means it set a frame range. The
entry now starts from kOfxStatReplyDefault, as the comment after its
dispatch already said it should, and every action it handles still
sets its own status.

Found by the new test host, which warns when a plugin answers
kOfxStatOK with nothing written.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
…num values

describeInContext offers only the colourspaces available in the
colour management style it expects the host to use, and the
"[Unspecified]" and "[Same as input]" entries were marked Basic. On a
host with no colour management every entry was skipped, so the three
string-choice parameters had no enum values and no default, which the
spec does not allow: the default must be one of the enums. The two
entries are now offered in every style, so each parameter has at
least that one choice.

The "[Unspecified]" entry's enum value was also NULL, passed to
propSetString as both an enum and the default whenever the host did
have colour management. A host that copies the value into a
std::string, as HostSupport does, cannot take that. It is now the
empty string, which getClipPreferences already reads as "no
preference".

Found by the new test host.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
drawText clipped the text it draws at the top and right of the image
but not at the bottom or left. Render places its three lines of text
100 pixels in from the left and top edges, each line 50 pixels below
the last at full scale, so on a frame under 200 pixels high a line starts
below row 0 and drawText wrote it before the start of the image data.
It now starts each loop at the image's edge.

Found by the new test host, which puts guard bytes around every image.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Render copied the source to the output with one memcpy of the source's
row bytes times its height, starting at the output's data pointer, as
if the output were the whole frame. It declares tile support, and a
tile's output image covers only the render window, so each tile got a
copy of the frame's bottom left corner and the copy ran on past the
end of the output buffer. When the source image was allocated just
after it, the two ranges overlapped: the intermittent crash that ASan
reports as memcpy-param-overlap. It now copies the render window a row
at a time, addressing each image through its own bounds and row bytes,
so it no longer needs the two row bytes to match, and it fills with
black whatever part of the window the source does not cover.

Tiled and whole-frame renders also differed for a second reason: the
text was placed relative to the output image, so every tile drew its
own copy in its own corner. The text is now placed relative to the
output's region of definition, which also gives the frame size it
reports in place of the source image's bounds, and drawn into the
render window, which drawText clips it to on all four sides since the
previous commit, so a tiled render is identical to a whole one. A
whole-frame render is unchanged.

Found by the new test host, with --tiles and --check-tiles.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
It set the output clip's depth from its parameter whatever the host said,
though a plugin may change a clip's depth only on a host that supports
multiple clip depths, and it answered kOfxStatOK even when its parameter
mapped to no depth and it had set nothing. It now keeps the host's
preferences in both cases, with kOfxStatReplyDefault. It also no longer
reads the host's support for multiple clip depths into an uninitialised
local, keeping it for the action instead.

Assisted-by: Claude Code / Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
@garyo
garyo marked this pull request as ready for review September 24, 2026 16:12

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant