Skip to content

Add enchant_dict_remove_from_session() #17393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Add enchant_dict_remove_from_session() #17393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3ef051
Add enchant_dict_remove_from_session()
nielsdos Jan 7, 2025
8786534
nulls
nielsdos Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ PHP NEWS
. Added Dom\Element::$outerHTML. (nielsdos)
. Added Dom\Element::insertAdjacentHTML(). (nielsdos)

- Enchant:
. Added enchant_dict_remove_from_session(). (nielsdos)

- Intl:
. Bumped ICU requirement to ICU >= 57.1. (cmb)
. IntlDateFormatter::setTimeZone()/datefmt_set_timezone() throws an exception
Expand Down
4 changes: 4 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ PHP 8.5 UPGRADE NOTES
- DOM:
. Added Dom\Element::insertAdjacentHTML().

- Enchant:
. Added enchant_dict_remove_from_session() to remove a word added to the
spellcheck session via enchant_dict_add_to_session().

- PGSQL:
. pg_close_stmt offers an alternative way to close a prepared
statement from the DEALLOCATE sql command in that we can reuse
Expand Down
16 changes: 16 additions & 0 deletions ext/enchant/enchant.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,22 @@ PHP_FUNCTION(enchant_dict_add_to_session)
}
/* }}} */

PHP_FUNCTION(enchant_dict_remove_from_session)
{
zval *dict;
const char *word;
size_t wordlen;
const enchant_dict *pdict;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "Op", &dict, enchant_dict_ce, &word, &wordlen) == FAILURE) {
RETURN_THROWS();
}

PHP_ENCHANT_GET_DICT;

enchant_dict_remove_from_session(pdict->pdict, word, wordlen);
}

/* {{{ whether or not 'word' exists in this spelling-session */
PHP_FUNCTION(enchant_dict_is_added)
{
Expand Down
2 changes: 2 additions & 0 deletions ext/enchant/enchant.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ function enchant_dict_add_to_personal(EnchantDictionary $dictionary, string $wor

function enchant_dict_add_to_session(EnchantDictionary $dictionary, string $word): void {}

function enchant_dict_remove_from_session(EnchantDictionary $dictionary, string $word): void {}

function enchant_dict_is_added(EnchantDictionary $dictionary, string $word): bool {}

/**
Expand Down
6 changes: 5 additions & 1 deletion ext/enchant/enchant_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions ext/enchant/tests/dict_remove_from_session.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
enchant_dict_remove_from_session() function
--EXTENSIONS--
enchant
--SKIPIF--
<?php
if (!is_array(enchant_broker_list_dicts(enchant_broker_init()))) die("skip no dictionary installed on this machine!");
?>
--FILE--
<?php
$broker = enchant_broker_init();
$dicts = enchant_broker_list_dicts($broker);
$newWord = 'myImaginaryWord';

$requestDict = enchant_broker_request_dict($broker, $dicts[0]['lang_tag']);
Copy link
Member

Choose a reason for hiding this comment

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

Mind adding a test which includes a null byte in the string?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh you're gonna love this.
Turns out that enchant crashes when you have a null byte in a string, for all the existing methods too...
So the fact that nulls aren't prevented is a bug since the introduction of the enchant extension.

Try it yourself!

<?php
$broker = enchant_broker_init();
$dicts = enchant_broker_list_dicts($broker);
$requestDict = enchant_broker_request_dict($broker, $dicts[0]['lang_tag']);
var_dump(enchant_dict_check($requestDict, "foo\0bar"));
libenchant-CRITICAL **: 19:54:21.646: string_substring: assertion '(offset + len) <= string_length' failed
And then a segfault

I guess I'll make a PR for lower branches to change the "s" to "p".

Copy link
Member

Choose a reason for hiding this comment

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

Lovely, I wonder how many bugs could be found by fuzzing extension functions with strings that contain null bytes.

@YuanchengJiang you might want to try this in your fuzzer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I rebased this and added a commit to properly handle the null bytes.
Will squash ofc on merge.


var_dump(enchant_dict_check($requestDict, $newWord));
enchant_dict_add_to_session($requestDict, $newWord);
var_dump(enchant_dict_check($requestDict, $newWord));
var_dump(enchant_dict_is_added($requestDict, $newWord));
enchant_dict_remove_from_session($requestDict, $newWord);
var_dump(enchant_dict_check($requestDict, $newWord));
var_dump(enchant_dict_is_added($requestDict, $newWord));

?>
--EXPECT--
bool(false)
bool(true)
bool(true)
bool(false)
bool(false)
2 changes: 2 additions & 0 deletions ext/enchant/tests/null_bytes.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ $two_params_dict = [
"enchant_dict_suggest",
"enchant_dict_add",
"enchant_dict_add_to_session",
"enchant_dict_remove_from_session",
"enchant_dict_is_added",
];

Expand Down Expand Up @@ -65,6 +66,7 @@ enchant_dict_check(): Argument #2 ($word) must not contain any null bytes
enchant_dict_suggest(): Argument #2 ($word) must not contain any null bytes
enchant_dict_add(): Argument #2 ($word) must not contain any null bytes
enchant_dict_add_to_session(): Argument #2 ($word) must not contain any null bytes
enchant_dict_remove_from_session(): Argument #2 ($word) must not contain any null bytes
enchant_dict_is_added(): Argument #2 ($word) must not contain any null bytes
enchant_broker_set_ordering(): Argument #2 ($tag) must not contain any null bytes
enchant_dict_store_replacement(): Argument #2 ($misspelled) must not contain any null bytes
Loading