Skip to content

Sanitize messages from python#5589

Merged
iorsh merged 2 commits intofontforge:masterfrom
Caellian:fix/sanitize-python-printing
Jul 21, 2025
Merged

Sanitize messages from python#5589
iorsh merged 2 commits intofontforge:masterfrom
Caellian:fix/sanitize-python-printing

Conversation

@Caellian
Copy link
Copy Markdown
Contributor

@Caellian Caellian commented Jul 21, 2025 •
edited
Loading

Previously passing strings containing '%' would cause fontforge to crash, this commit makes fontforge emit a warning instead and escape those characters.

Closes #45.

Testing

Tested with:

import fontforge

fontforge.logWarning("This should be just fine as always...")
fontforge.logWarning("Ooops, I forgot to escape % so there might be trouble")
fontforge.logWarning("This case might be a bit different:%")
fontforge.logWarning("A % string with % very%many % unescaped %%% formats")

Output:

This should be just fine as always...
logError called with a string containing unescaped format character(s): 'Ooops, I forgot to escape % so there might be trouble'
Ooops, I forgot to escape % so there might be trouble
logError called with a string containing unescaped format character(s): 'This case might be a bit different:%'
This case might be a bit different:%
logError called with a string containing unescaped format character(s): 'A % string with % very%many % unescaped %%% formats'
A % string with % very%many % unescaped %% formats

Type of change

  • Bug fix
  • Non-breaking change

Caellian
Caellian commented Jul 21, 2025
View reviewed changes
Comment thread fontforge/python.c
Caellian
Caellian commented Jul 21, 2025
View reviewed changes
Comment thread fontforge/python.c
Caellian
Caellian commented Jul 21, 2025
View reviewed changes
Comment thread fontforge/python.c Outdated
@Caellian
Copy link
Copy Markdown
Contributor Author

Caellian commented Jul 21, 2025

cc @skef

@Caellian
Sanitize messages from python
7861f85 
Previously passing strings containing '%' would cause fontforge to
crash, this commit makes fontforge emit a warning instead and escape
those characters.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian force-pushed the fix/sanitize-python-printing branch from e9974af to 7861f85 Compare July 21, 2025 09:42
iorsh
iorsh reviewed Jul 21, 2025
View reviewed changes
Comment thread fontforge/python.c Outdated
Comment thread fontforge/python.c Outdated
Comment thread fontforge/python.c Outdated
Comment thread fontforge/python.c Outdated
@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Jul 21, 2025

Testing

Tested with:

import fontforge

fontforge.logWarning("This should be just fine as always...")
fontforge.logWarning("Ooops, I forgot to escape % so there might be trouble")
fontforge.logWarning("This case might be a bit different:%")
fontforge.logWarning("A % string with % very%many % unescaped %%% formats")

[Optional] Please add this code snippet to a unit test (see test*.py examples in tests)

@Caellian Caellian force-pushed the fix/sanitize-python-printing branch 2 times, most recently from 17ba00b to a4326f5 Compare July 21, 2025 19:54
iorsh
iorsh reviewed Jul 21, 2025
View reviewed changes
Comment thread tests/test1022.py
@Caellian Caellian force-pushed the fix/sanitize-python-printing branch from a4326f5 to 8bf7659 Compare July 21, 2025 20:14
@Caellian
Apply requested improvements
8f95335 
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian force-pushed the fix/sanitize-python-printing branch from 8bf7659 to 8f95335 Compare July 21, 2025 20:15
@Caellian Caellian requested a review from iorsh July 21, 2025 20:16
iorsh
iorsh approved these changes Jul 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@iorsh iorsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@iorsh iorsh merged commit aaecad7 into fontforge:master Jul 21, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iorsh iorsh iorsh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Interpolation crash

2 participants

@Caellian @iorsh