Skip to content
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.

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

Debug assertion triggered on empty doctype #608

Closed
Tpt opened this issue Jun 3, 2023 · 2 comments · Fixed by #612
Closed

Debug assertion triggered on empty doctype #608

Tpt opened this issue Jun 3, 2023 · 2 comments · Fixed by #612

Comments

@Tpt
Copy link
Contributor

Tpt commented Jun 3, 2023

Thank you for quick-xml and sorry for comming back with an other bug.

When parsing the file <!DoctyPE> using rusc "dev" profile the assertion "DocType must have a name" is triggered. In the "release" profile the assertion is not triggered and the parser returns an empty Event::Doctype()

Test failing in "dev" profile and passing in "release" profile:

#[test]
fn reproduce() {
    let data = b"<!DoctyPE>";
    let mut reader = Reader::from_reader(data.as_slice());
    let mut buf = Vec::new();
    loop {
        match reader.read_event_into(&mut buf) {
            Ok(Event::Eof) => break,
            Ok(Event::DocType(e)) => assert!(e.is_empty()), // assertion only executed in "release" mode
            _ => unreachable!()
        }
    }
}
Tpt added a commit to oxigraph/oxigraph that referenced this issue Jun 4, 2023
Tpt added a commit to oxigraph/oxigraph that referenced this issue Jun 5, 2023
@Mingun
Copy link
Collaborator

Mingun commented Jun 10, 2023

Yes, I think, we should either to parse this correctly into an event or return a normal error if this violates XML rules

@Tpt
Copy link
Contributor Author

Tpt commented Jun 10, 2023

XML 1.0 grammar requires at least one "Name" into the Doctype declaration. So, this case seems to violate XML rules.

Tpt added a commit to Tpt/quick-xml that referenced this issue Jun 11, 2023
The parser was crashing with debug assertions enabled but passing with debug assertions disabled

Closes tafia#608
Tpt added a commit to Tpt/quick-xml that referenced this issue Jun 11, 2023
The parser was crashing with debug assertions enabled but passing with debug assertions disabled

Closes tafia#608
Tpt added a commit to Tpt/quick-xml that referenced this issue Jun 11, 2023
The parser was crashing with debug assertions enabled but passing with debug assertions disabled

Closes tafia#608
Mingun pushed a commit to Tpt/quick-xml that referenced this issue Jun 11, 2023
The parser was crashing with debug assertions enabled but passing with debug assertions disabled

Closes tafia#608
crapStone added a commit to Calciumdibromid/CaBr2 that referenced this issue Jun 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [quick-xml](https://github.com/tafia/quick-xml) | dependencies | minor | `0.28.2` -> `0.29.0` |

---

### Release Notes

<details>
<summary>tafia/quick-xml</summary>

### [`v0.29.0`](https://github.com/tafia/quick-xml/blob/HEAD/Changelog.md#&#8203;0290----2023-06-13)

[Compare Source](tafia/quick-xml@v0.28.2...v0.29.0)

##### New Features

-   [#&#8203;601]: Add `serde_helper` module to the crate root with some useful utility
    functions and document using of enum's unit variants as a text content of element.
-   [#&#8203;606]: Implement indentation for `AsyncWrite` trait implementations.

##### Bug Fixes

-   [#&#8203;603]: Fix a regression from [#&#8203;581] that an XML comment or a processing
    instruction between a <!DOCTYPE> and the root element in the file brokes
    deserialization of structs by returning `DeError::ExpectedStart`
-   [#&#8203;608]: Return a new error `Error::EmptyDocType` on empty doctype instead
    of crashing because of a debug assertion.

##### Misc Changes

-   [#&#8203;594]: Add a helper macro to help deserialize internally tagged enums
    with Serde, which doesn't work out-of-box due to serde limitations.

[#&#8203;581]: tafia/quick-xml#581

[#&#8203;594]: tafia/quick-xml#594

[#&#8203;601]: tafia/quick-xml#601

[#&#8203;603]: tafia/quick-xml#603

[#&#8203;606]: tafia/quick-xml#606

[#&#8203;608]: tafia/quick-xml#608

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTguMCIsInVwZGF0ZWRJblZlciI6IjM1LjExOC4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1940
Reviewed-by: crapStone <crapstone01@gmail.com>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants