Skip to content

Store and remember the quote type of attribute values - #1114

Closed
malteskoruppa wants to merge 1 commit into
jhy:masterfrom
malteskoruppa:store_attribute_value_quote_type
Closed

malteskoruppa wants to merge 1 commit into
jhy:masterfrom
malteskoruppa:store_attribute_value_quote_type

Conversation

@malteskoruppa

Copy link
Copy Markdown

I'm using jsoup to parse HTML documents and perform some analysis on them.

Unfortunately, I discovered that jsoup does not remember the quote type used for attribute values during parsing. In other words, parsing the following strings:

Document foo = Jsoup.parse("<html><body><a name=\"value\"></body></html>");
Document bar = Jsoup.parse("<html><body><a name='value'></body></html>");
Document baz = Jsoup.parse("<html><body><a name=value></body></html>");

produces three identical documents, with no way to retrieve the information what quote types where used for the value of the name atrribute of the a element (double quotes, single quotes, or no quotes).

Though I do understand how one may want to abstract away such information in a DOM tree, I think it would be useful to at least make the information optionally available to the user. For instance, when analyzing an HTML document for potential XSS weaknesses, this information can be highly relevant. I see that jsoup already does a few things to help users in preventing XSS vulnerabilties, so making this information available to users would be in line with that spirit.

This PR makes it so that each attribute's value's quote type is stored in each attribute during parsing, and this information is made available to the user via an appropriate getter method that returns an enum element (DOUBLE_QUOTED, SINGLE_QUOTED, or UNQUOTED).

I personally dearly need this information, so it would be great if this PR could make it into a future release. I am new to jsoup, so I may have forgotten about some other locations where code should be changed. If that is the case, please let me know, I can obviously change things. For instance, this information could potentially be used to improve the html() and similar methods to produce output that is closer related to the originally parsed source code by re-using the original quote types for each attribute during printing. (It would also allow users to manipulate and control the quote types used for each attribute). For the moment I did not do so, and all attribute name/value pairs are still printed out using double quotes as before, as I was not sure whether such a behavior is desired and what consequences it may have in other applications.

@arturgspb

Copy link
Copy Markdown

I have a similar problem, I need to change html and attributes with JSON content.

After doc.html() I have

elem-attrs="{&quot;type&quot;:&quot;daterange&quot;}"

but I expected

elem-attrs='{"type":"daterange"}'

@arturgspb

Copy link
Copy Markdown

@malteskoruppa you have custom build with fix in Maven?

@arturgspb

arturgspb commented Dec 18, 2018 •

Copy link
Copy Markdown

@jhy I try this: StringEscapeUtils.unescapeHtml but return is:

elem-attrs="{"type":"daterange"}"

Double quote with inner double quote.
Maybe you can give me help variants?

@malteskoruppa

Copy link
Copy Markdown
Author

@arturgspb If I understand correctly, what you need is for jsoup to print an attribute value enclosed in single quotes instead of in double quotes.

What this PR does is something slightly different: With this PR, when parsing HTML, the resulting Document remembers, for each attribute value, how it was quoted. In other words, this PR is about reading attribute values, whereas what you want is another behavior when writing attribute values.

As far as I know, jsoup always outputs attribute values in double quotes. That's why I suggested that after this PR; the stored information on how attribute values were quoted in an original document could be reused to use the same quotation marks when outputting the document again.

Anyhow, you'll have to change the source code if you want to change how attribute values are quoted during printing. Shouldn't be too difficult though. :-) Simply look for the place where the double quotes are printed (here) and replace them with single quotes. Of course, then you'll print all attribute values in single quotes. If you want to replace them with whatever the original document had, then you can use this PR and use the stored information to print the original quotation marks at said location.

@BenCrulis

Copy link
Copy Markdown

Is there any update on this?

I have the same problem, an HTML page has JSON enclosed in single quotes in some attributes but rendered as escaped HTML enclosed in double quotes.
I need the full document to be rendered as close as the original, as I have very little control on the javascript that will be ran on the page and it expects unescaped JSON. The page itself also seems to be drawn slightly differently by the browsers compared to the original page, which is problematic.

It appears that quite a few people had the same problems over the years on stackoverflow, the questions that received answers were provided with no satisfying solution for my use case.

This PR seems to be headed in the right direction to fix this. Maybe we should also fill an issue.

@jhy

jhy commented Jul 30, 2024 •

Copy link
Copy Markdown
Owner

Sorry for the late reply on this. I am not moving ahead with the PR. Primarily, the Attributes object is designed to minimize its memory consumption, to keep the DOM as light as possible. Adding this overhead to every element in every DOM for a presentational output is not warranted, IMO.

I think there may be more utility in adding a method to the pretty-printer that auto minified attributes during serializing. E.g. if there's no spaces, no quotes are needed. And if quotes are needed, choose " or ' based on which would lead to the least escapes.

And for folks that are saying this is required for embedding JSON in the DOM. It is not. Something else is going wrong. I can't say what it is because you didn't show a worked example. Please go ahead and file an issue with a worked test case. My guess is that you're treating text as HTML, or HTML as text.

@jhy jhy closed this Jul 30, 2024
@malteskoruppa

Copy link
Copy Markdown
Author

@jhy Thank you for your comments, even if they come late. Better late than never! πŸ™‚

Just for the record, for me as an author of this PR, the rationale for this PR has nothing whatsoever to do with either presentational output or embedding JSON.

Rather, my use case is about security (XSS) analysis. As stated in my original comment on this PR:

I think it would be useful to at least make the information optionally available to the user. For instance, when analyzing an HTML document for potential XSS weaknesses, this information can be highly relevant. I see that jsoup already does a few things to help users in preventing XSS vulnerabilties, so making this information available to users would be in line with that spirit.

This PR makes it so that each attribute's value's quote type is stored in each attribute during parsing, and this information is made available to the user via an appropriate getter method that returns an enum element (DOUBLE_QUOTED, SINGLE_QUOTED, or UNQUOTED).

More precisely, when user input is reflected in an HTML attribute, but the user input has been sanitized to not contain single or double quotes, then the user input may still escape the semantics of the underlying HTML code if the attribute was unquoted.

For illustration: In a nutshell, I am analyzing some server-side code that could look like so (I am doing this for different languages, but I will use PHP as an example):

<?php
$input = htmlspecialchars($_GET["foo"], ENT_QUOTES);
?>
<html><body>
Hello <img href="..." alt=<?= $input ?>>!
</html></body>

The above code is vulnerable to an XSS injection, since the alt tag does not use quotes, so using PHP's htmlspecialchars is useless.

Using advanced techniques, I transform the above code into something like this:

<html><body>
Hello <img href="..." alt=USER_INPUT_WITHOUT_QUOTES>!
</html></body>

Then, I parse this with jsoup to see if there may be a vulnerability. In this case there is, since, even if the user input cannot contain qutoes, the alt attribute is actually unquoted.

In reality, it's a lot more complicated and invovled than that, and I am oversimplifying. πŸ™‚

Granted, such analysis is a fairly special use case, so of course, I respect your decision to prioritize a lighter memory footprint. It is simply a matter of what you need jsoup for. πŸ™‚ If your focus is on pretty-printing/presentation of output (which is what I infer from your answer), then your decision is perfectly valid. However, it does mean that vanilla jsoup is not well-suited to perform context-sensitive security analysis of HTML documents (to detect potential XSS vulnerabilities), such as outlined above.

@jhy

jhy commented Jul 30, 2024

Copy link
Copy Markdown
Owner

Thank you for your follow-up and for sharing your use case in detail.

Take a look at the new-ish Attributes.sourceRange() method. This method allows you to obtain the position in the source where each attribute name and value was parsed from. By examining the characters surrounding the attribute's value source range, you should be able to determine the original quote type.

When tracking is enabled the ranges are stored as internal attribute values, so the memory use is opt-in. This approach keeps the DOM lightweight unless such detailed tracking is required.

I also added Attributes.userData(String key, Object value) to allow users to store arbitrary objects along with an element. These don't appear in serialization. This was added to help support more custom requirements like yours in a somewhat general way.

Source range tracking with this fidelity is fairly unique to jsoup for HTML and XML parsers, afaik. Which makes it a powerful tool for tasks such as security analysis. (And of course XSS protection via the Safelist Cleaner, which is jsoup's fundamental XSS safety tool.)

Please feel free to reach out if using the source range doesn't work out or if you see ways to improve it.

@malteskoruppa

Copy link
Copy Markdown
Author

Hey @jhy, thanks a lot for the input! This does indeed seem like a very good alternative. Thank you very much for the feedback. πŸ™‚

Repository owner locked as resolved and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants