Bug 24379 - xmlSetStructuredErrorFunc() breaks SAX2 error handler
Summary: xmlSetStructuredErrorFunc() breaks SAX2 error handler
Status: CLOSED FIXED
Alias: None
Product: Sisyphus
Classification: Development
Component: libxml2 (show other bugs)
Version: unstable
Hardware: all Linux
: P3 blocker
Assignee: Alexey Shabalin
QA Contact: qa-sisyphus
URL: https://bugzilla.gnome.org/show_bug.c...
Keywords:
Depends on:
Blocks: 23900 24072 24106 24107
  Show dependency tree
 
Reported: 2010-10-21 10:52 MSD by Sir Raorn
Modified: 2011-01-13 03:24 MSK (History)
11 users (show)

See Also:


Attachments
t.c (1.44 KB, text/plain)
2010-12-29 14:18 MSK, Sir Raorn
no flags Details
a patch by Sergei Epiphanov (803 bytes, patch)
2010-12-30 18:40 MSK, Michael Shigorin
no flags Details | Diff
libxml-error.patch (1.05 KB, patch)
2010-12-31 02:56 MSK, Alexey Gladkov
no flags Details | Diff
testcase (1.67 KB, text/plain)
2011-01-03 00:52 MSK, Alexey Gladkov
no flags Details
libxml-error.patch (1.97 KB, patch)
2011-01-03 01:14 MSK, Alexey Gladkov
no flags Details | Diff
Исправленый testcase (1.64 KB, text/plain)
2011-01-09 10:01 MSK, serpiph
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sir Raorn 2010-10-21 10:52:02 MSD
В структуре _xmlSAXHandler (xmlParserCtxt->sax) есть поле serror, указывающий на функцию обработчик ошибок.  В эту функцию должен передаваться указатель на пользовательские данные (xmlParserCtxt->userData) и указатель на xmlLastError.

Однако, если когда-то была вызвана функция xmlSetStructuredErrorFunc(), в этот хендлер передаётся указатель, который передавался в xmlSetStructuredErrorFunc().

http://git.altlinux.org/people/at/packages/?p=libxml2.git;a=blob;f=error.c, строки 595-616.
Comment 1 Sir Raorn 2010-10-29 14:13:03 MSD
Блокирует сборку libxml-ruby и всего RoR.
Comment 2 Dmitry V. Levin 2010-10-29 14:15:45 MSD
(In reply to comment #1)
> Блокирует сборку libxml-ruby и всего RoR.

Сборку блокирует сборочница, а вовсе не какие-то там жалкие пакеты.
Пожалуйста, если у тебя есть возможность, то попробуй исправить сам.
Comment 3 Dmitry V. Levin 2010-11-05 22:18:45 MSK
Сейчас у нас нет полноценного мейнтейнера пакета libxml2, который мог бы проанализировать ситуацию и дать обоснованное заключение.

Я сейчас тупо обновляю libxml2 до новой версии 2.7.8, в которой исправлены crash-баги.

Поручиться в том, что libxml2 работает неправильно, я не могу.
Разбираться в правильности реализации API у меня сейчас возможности нет.

Ввиду вышеперечисленного предлагаю жаловаться Daniel Veillard в апстрим (можно и в Федорную багзиллу, поскольку он там является мейнтейнером libxml2).
Comment 4 Radik Usupov 2010-11-26 11:14:14 MSK
А может все-таки что-нибудь попробуем сделать?
Не хотелось бы вылета RoR.
Comment 5 Dmitry V. Levin 2010-12-27 16:33:20 MSK
(In reply to comment #4)
> А может все-таки что-нибудь попробуем сделать?
> Не хотелось бы вылета RoR.

У нас, в Fedora и Debian практически одинаковые пакеты libxml2.
Поскольку для них API, реализованный в libxml2, не является препятствием для упаковки libxml-ruby, напрашивается очевидный вывод: libxml2 не блокирует сборку libxml-ruby и всего остального.
Comment 6 Sir Raorn 2010-12-27 17:06:48 MSK
Конечно libxml2 не блокирует сборку libxml-ruby, если этот libxml-ruby вообще не собирать.

Я понял, скриптовые языки в репозитарии никому нахрен не упёрлись.  Спасибо за конструктивный ответ.
Comment 7 Dmitry V. Levin 2010-12-27 17:32:36 MSK
(In reply to comment #6)
> Конечно libxml2 не блокирует сборку libxml-ruby, если этот libxml-ruby вообще
> не собирать.

Как же, интересно, _все_ остальные собирают libxml-ruby и не страдают от того, что у libxml2 якобы неправильно реализованный API?

> Я понял, скриптовые языки в репозитарии никому нахрен не упёрлись.  Спасибо за
> конструктивный ответ.

Похоже, что бывший мейнтейнер libxml-ruby просто забил на свои пакеты и хочет напоследок свалить с больной головы на здоровую.  Ну что же, спасибо, г-н бывший мейнтейнер. :(
Comment 8 Sir Raorn 2010-12-27 17:39:22 MSK
(In reply to comment #7)
> Как же, интересно, _все_ остальные собирают libxml-ruby и не страдают от того,
> что у libxml2 якобы неправильно реализованный API?
"Все остальные" не заморачиваются с тестами.

> Похоже, что бывший мейнтейнер libxml-ruby просто забил на свои пакеты и хочет
> напоследок свалить с больной головы на здоровую.  Ну что же, спасибо, г-н
> бывший мейнтейнер. :(
Похоже, что некоторые неполноценные* мейнтейнеры реагируют только на истерику, при чём инициированную только другими неполноценными* мейнтейнерами.  Бывшему мейнтейнеру libxml-ruby не очень приятно что-то делать в такой обстановке.  Ну что же, спасибо, г-н неполноценный* мейнтейнер. :(

*) bug 24379 comment 3
Comment 9 Dmitry V. Levin 2010-12-27 22:58:31 MSK
(In reply to comment #8)
> (In reply to comment #7)
> > Как же, интересно, _все_ остальные собирают libxml-ruby и не страдают от того,
> > что у libxml2 якобы неправильно реализованный API?
> "Все остальные" не заморачиваются с тестами.

Главное правило разработчика свободного софта "тебе надо - тебе и делать" не забыл?  Это твои тесты, ты и заморачивайся.
К сожалению, помогать тебе в этом вопросе я больше не хочу.
Comment 10 Sir Raorn 2010-12-27 23:07:57 MSK
(In reply to comment #9)
> Это твои тесты, ты и заморачивайся.
Мне понимать это как "ты что, самый умный что ли - тесты при сборке запускать?" ?
Comment 11 Dmitry V. Levin 2010-12-27 23:13:03 MSK
(In reply to comment #10)
> (In reply to comment #9)
> > Это твои тесты, ты и заморачивайся.
> Мне понимать это как "ты что, самый умный что ли - тесты при сборке запускать?"
> ?

Это понимать как "сам выясняй, почему тесты не проходят, сам готовь патч, сам объясняй апстриму, почему этот патч правильный, и тогда все лавры достанутся тебе".
Comment 12 AEN 2010-12-27 23:15:10 MSK
(В ответ на комментарий №10)
> (In reply to comment #9)
> > Это твои тесты, ты и заморачивайся.
> Мне понимать это как "ты что, самый умный что ли - тесты при сборке запускать?"
> ?

Сперва было то же самое в форме просьбы:
#2: ldv@: "Пожалуйста, если у тебя есть возможность, то попробуй исправить
сам."
Comment 13 Sir Raorn 2010-12-27 23:56:45 MSK
(In reply to comment #12)
> Сперва было то же самое в форме просьбы:
> #2: ldv@: "Пожалуйста, если у тебя есть возможность, то попробуй исправить
> сам."
У меня нет такой возможности.  Невозможно убедить неполноценного* мейнтейнера исправить баг в своём пакете.  И невозможно выслушивать нравоучения на ломаном английском от другого неполноценного* мейнтейнера по поводу точечек в Release и использования слова "NMU" в %changelog.

Впрочем, всё это уже неважно:

$ ssh git.alt help
ssh: Permission denied (publickey).

*) bug 24379 comment 3
Comment 14 Dmitry V. Levin 2010-12-28 01:15:18 MSK
(In reply to comment #13)
> (In reply to comment #12)
> > Сперва было то же самое в форме просьбы:
> > #2: ldv@: "Пожалуйста, если у тебя есть возможность, то попробуй исправить
> > сам."
> У меня нет такой возможности.

У меня тем более нет такой возможности, поскольку УМВР.
Comment 15 Alexey Gladkov 2010-12-29 14:18:22 MSK
Я попробую разобраться.
Comment 16 Sir Raorn 2010-12-29 14:18:43 MSK
Created attachment 4726 [details]
t.c

Вот тебе testcase.  Наслаждайся.

$ gcc -Wall -o t t.c `pkg-config --cflags --libs libxml-2.0`
$ ./t
usage: ./t XML [SET_ERR_FUNC]
$ ./t "<tag"                                                
SEC: 0x0xdeadbeef
$ ./t "<tag" 1
SEC: 0x(nil)
Comment 17 Michael Shigorin 2010-12-30 18:40:29 MSK
Created attachment 4728 [details]
a patch by Sergei Epiphanov

http://lists.altlinux.org/pipermail/devel/2010-December/187512.html и около
Comment 18 Alexey Gladkov 2010-12-31 02:56:12 MSK
Created attachment 4731 [details]
libxml-error.patch

Я бы сделал вот так. Сломаться ничего не должно, но нужно проверить.
Comment 19 Sir Raorn 2010-12-31 03:20:28 MSK
(In reply to comment #18)
> Я бы сделал вот так. Сломаться ничего не должно, но нужно проверить.
Как себя ведёт testcase?
Comment 20 Alexey Gladkov 2010-12-31 11:58:12 MSK
Код у автора очень путанный. В идеале __xmlRaiseError() совсем бы переделать.

$ ./t "<tag" 
SEC: 0x0xdeadbeef

$ ./t "<tag" 1
SEC: 0x0xbadf00d
Comment 21 Sir Raorn 2010-12-31 12:11:47 MSK
(In reply to comment #20)
> $ ./t "<tag" 
> SEC: 0x0xdeadbeef
> $ ./t "<tag" 1
> SEC: 0x0xbadf00d

Оба раза должен вызываться structured_error_callback (SEC) с параметром 0xdeadbeef (ctxt->userData).
Comment 22 Alexey Gladkov 2010-12-31 12:40:07 MSK
(В ответ на комментарий №21)
> Оба раза должен вызываться structured_error_callback (SEC) с параметром
> 0xdeadbeef (ctxt->userData).

А когда должно вызываться structuredErrorFunc ?
Comment 23 Alexey Gladkov 2010-12-31 12:48:02 MSK
xmlSetStructuredErrorFunc() - Function to reset the handler and the error context for out of context structured error messages.

Он и ресетит. На мой взгляд ошибка в том, что не была вызвана structuredErrorFunc().
Comment 24 Sir Raorn 2010-12-31 12:48:02 MSK
В данном примере думаю что вообще не должно.  Потому как есть ctxt->sax->serror (или как там это поле называется).  Это видимо используется другими парсерами.
Comment 25 Alexey Gladkov 2010-12-31 13:04:32 MSK
(В ответ на комментарий №24)
> В данном примере думаю что вообще не должно.  Потому как есть ctxt->sax->serror
> (или как там это поле называется).  Это видимо используется другими парсерами.

Судя по коду функции structured callback channel имеет приоритет над 
callback channel, который берётся из контекста.

Об этом написано тут:

http://git.gnome.org/browse/libxml2/commit/?id=1de382eb061b70b07040b3932c4b6082eb3ded79

error.c: when defined use the structured error report over any generic one
Comment 26 Alexey Gladkov 2011-01-03 00:52:17 MSK
Created attachment 4732 [details]
testcase

Чуть-чуть параметризовал тесткейс.
Теперь можно проверить сочетания обработчиков.
Comment 27 Alexey Gladkov 2011-01-03 01:14:40 MSK
Created attachment 4733 [details]
libxml-error.patch

Вот патч, который исправляет libxml2.

На самом деле для апстрима тут три патча (зависит от вменяемости апстрима). Дело в том, что авторы просто забывают чистить старый код и плодят одни костыли поверх других.

$ ./t false false
$

$ ./t true false
Setting xmlSetStructuredErrorFunc()
SEF: 0xbadf00d

$ ./t false true
Setting ctxt->sax->serror
SEC: 0xdeadbeef

$ ./t true true
Setting xmlSetStructuredErrorFunc()
Setting ctxt->sax->serror
SEC: 0xdeadbeef

Также умиляет:

$ grep -m1 -A6 test_xmlSetStructuredErrorFunc testapi.c
test_xmlSetStructuredErrorFunc(void) {
    int test_ret = 0;


    /* missing type support */
    return(test_ret);
}

$ stat -c '%s' testapi.c
1423394
Comment 28 Alexey Gladkov 2011-01-08 14:26:22 MSK
Автор патчей другой. Ему и карты в руки.
Comment 29 Michael Shigorin 2011-01-08 14:45:24 MSK
В смысле ldv@?
Comment 30 serpiph 2011-01-09 10:01:48 MSK
Created attachment 4742 [details]
Исправленый testcase

В последнем testcase есть ошибки, прикладываю исправленный.
Comment 31 Dmitry V. Levin 2011-01-09 11:22:52 MSK
(In reply to comment #30)
> Created an attachment (id=4742) [details]
> Исправленый testcase

Сергей, техническое обсуждение уже давно переехало в кулуары,
а ссылки на testcase и патчи можно найти по адресу
https://bugzilla.gnome.org/show_bug.cgi?id=638618
Comment 32 Repository Robot 2011-01-13 03:24:43 MSK
libxml2-1:2.7.8-alt4 -> sisyphus:

* Wed Jan 12 2011 Dmitry V. Levin <ldv@altlinux> 1:2.7.8-alt4
- Fixed structured error handlers interoperability regression introduced
  between 2.7.3 and 2.7.4 libxml2 releases (closes: #24379).