Skip to content

Parse percentage and invalid *-opacity values without crashing (#422)#453

Open
apoorvdarshan wants to merge 1 commit into
Kozea:mainfrom
apoorvdarshan:fix/issue-422-opacity-percentage
Open

Parse percentage and invalid *-opacity values without crashing (#422)#453
apoorvdarshan wants to merge 1 commit into
Kozea:mainfrom
apoorvdarshan:fix/issue-422-opacity-percentage

Conversation

@apoorvdarshan

Copy link
Copy Markdown

Fixes #422

Root cause

opacity, stroke-opacity and fill-opacity are read in surface.py with a
raw float():

opacity = float(node.get('opacity', 1))                # surface.py:325
stroke_opacity = float(node.get('stroke-opacity', 1))  # surface.py:415
fill_opacity = float(node.get('fill-opacity', 1))      # surface.py:416

Any value that is not a plain number makes float() raise ValueError, which
propagates out of draw() and aborts the whole render. This covers both the
percentage syntax allowed for these properties (e.g. 50%) and non-numeric
values such as the null reported in the issue.

Reproduction

import cairosvg

svg = ('<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10">'
       '<rect width="10" height="10" fill="red" fill-opacity="50%"/></svg>')
cairosvg.svg2png(bytestring=svg.encode())

Observed on main (identical for fill-opacity="null", opacity="50%" and
stroke-opacity="50%"):

ValueError: could not convert string to float: '50%'

With this change the same input renders normally, and fill-opacity="50%"
produces byte-for-byte the same PNG as fill-opacity="0.5".

Fix

Add a small helpers.parse_opacity() that accepts a number or a percentage,
clamps the result to [0, 1] (as the SVG spec requires for opacity values),
and falls back to the default for anything it cannot parse — so an invalid
*-opacity attribute never aborts the render. The three reads in surface.py
now go through it. The change is localized to the opacity parsing and does not
touch any other behaviour; valid numeric opacities keep rendering exactly as
before.

Tests

Added test_opacity_percentage and test_opacity_invalid in
cairosvg/test_api.py (parametrized over opacity, fill-opacity and
stroke-opacity). They assert that a percentage renders identically to the
equivalent fraction and that an invalid value falls back to opaque instead of
crashing. Both fail on the current code with the ValueError above and pass
with this change. The full cairosvg/test_api.py suite passes (13 tests), and
flake8 / isort are clean on the changed files.

Disclosure: prepared with AI assistance; reviewed and verified locally.

opacity, stroke-opacity and fill-opacity were read with a raw float(),
so a percentage such as '50%' or a non-numeric value such as 'null'
raised ValueError and aborted the whole render.

Route the three reads through a new helpers.parse_opacity() that accepts
a number or a percentage, clamps the result to [0, 1] per the SVG spec,
and falls back to the default for unparseable values.

Fixes Kozea#422
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.

ValueError: could not convert string to float: 'null'

1 participant