Intent to remove <isindex>.

[Mike West]

Primary eng (and PM) emails

mk...@chromium.org

Summary

I would like to remove "support" for the obsolete isindex element[1] from Blink. I put support in quotes because what we currently do with the tag is strange, and doesn't seem to actually support it's usage in the wild.

Motivation

Currently, blink parses the isindex element by magically inserting a form and input element, along with some explanatory text (see HTMLTreeBuilder::processIsindexStartTagForInBody). This ends up looking like the "This is a searchable index. ..." at the top of [2], for example.

Typing something into that field and submitting the form reloads the page with the search query appended as a query string. In my testing, this has correctly searched for something on exactly 0 pages.

Given that the element is interpreted in a way that injects an unexpected form into a document, there is some non-trivial risk of a content injection attack making use of this element that probably isn't high on folks' lists of elements to filter[3]

[1]: http://www.whatwg.org/specs/web-apps/current-work/multipage/obsolete.html#isindex-0

[2]: http://www.ug.ru/old/95.25/250902.htm

[3]: http://www.thespanner.co.uk/2008/08/26/new-xss-vector/

[4]: http://www.w3.org/MarkUp/html-spec/html.dtd

Usage information from UseCounter

I don't have UseCounter data (and it's nontrivial to get, since the replacement happens somewhere where we don't currently have easy access to the counter mechanism). Grepping for the text through Google's search index reveals minimal usage: 756 results total on 341 unique domains (this includes pages like [4] that merely mention the tag; the tool's syntax is strange, and I don't know how to filter those out).

Compatibility Risk

Firefox supports isindex in exactly the same way that Blink does. Pages that rely on this behavior will break. So far I haven't found any.

Row on feature dashboard?

No.

WDYT?

-mike

[TAMURA, Kent]

This behavior is defined by the HTML standard, and it's usable if <isindex> has action attribute.

I don't think we can remove it without UseCounter data.

--
TAMURA Kent
Software Engineer, Google

[TAMURA, Kent]

http://www.whatwg.org/specs/web-apps/current-work/multipage/tree-construction.html#isindex

[Daniel Bratell]

On Mon, 02 Dec 2013 09:22:37 +0100, Mike West <mk...@chromium.org> wrote:

Primary eng (and PM) emails

mk...@chromium.org

Summary

I would like to remove "support" for the obsolete isindex element[1] from Blink. I put support in quotes because what we currently do with the tag is strange, and doesn't seem to actually support it's usage in the wild.

Motivation

Currently, blink parses the isindex element by magically inserting a form and input element, along with some explanatory text (see HTMLTreeBuilder::processIsindexStartTagForInBody). This ends up looking like the "This is a searchable index. ..." at the top of [2], for example.

Typing something into that field and submitting the form reloads the page with the search query appended as a query string. In my testing, this has correctly searched for something on exactly 0 pages.

Given that the element is interpreted in a way that injects an unexpected form into a document, there is some non-trivial risk of a content injection attack making use of this element that probably isn't high on folks' lists of elements to filter[3]

This sounds like it's doing what it should do.

<isindex> is the predecessor of web forms and pretty ugly, and mostly just exists on historical sites from mid 90s. Those are not very many but they might still be of interest.

Even if it's in the obsolete section of the HTML specification, it's still in the HTML standard. I don't think it's setting a good precedent to go around dropping support for various parts of the HTML standard just because we think they are ugly. If I recall correctly the cost of supporting <isindex> is also very low since it just expands in the parser and no other part of the browser code needs to care about it.

/Daniel

[Mike West]

Hi Kent, Daniel!

I agree that our implementation is as-specified. I also agree that it matches Firefox, Safari, and probably IE. I've also skimmed through a number of the historical sites you're referring to, and haven't found any that reacted to a search driven through the injected form. 

You're also correct that the cost of support is minimal (~40 lines of code that haven't been touched in forever: https://codereview.chromium.org/96653004/). My motivation is neither ugliness nor maintenance, but "Hey, this unexpectedly injected a form into my page, bypassing my clever `s/<form//` regex protection!" Recent comments like https://twitter.com/0x6D6172696F/status/406006348577374208 bother me. :)

-mike

--
Mike West <mk...@google.com>

Google+: https://mkw.st/+, Twitter: @mikewest, Cell: +49 162 10 255 91

Google Germany GmbH, Dienerstrasse 12, 80331 München, Germany
Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Graham Law, Christine Elizabeth Flores

[Daniel Bratell]

On Mon, 02 Dec 2013 10:03:09 +0100, Mike West <mk...@google.com> wrote:

Hi Kent, Daniel!

I agree that our implementation is as-specified. I also agree that it matches Firefox, Safari, and probably IE. I've also skimmed through a number of the historical sites you're referring to, and haven't found any that reacted to a search driven through the injected form. 

You're also correct that the cost of support is minimal (~40 lines of code that haven't been touched in forever: https://codereview.chromium.org/96653004/). My motivation is neither ugliness nor maintenance, but "Hey, this unexpectedly injected a form into my page, bypassing my clever `s/<form//` regex protection!" Recent comments like https://twitter.com/0x6D6172696F/status/406006348577374208 bother me. :)

Unfortunately that one is protected, but I can imagine that <isindex> is mistreated somewhere on the web. That tag is included as an especially interesting tag in several of the fuzzers I've seen and before Presto used the current HTML5 parser it wasn't unheard of that <isindex> triggered some bug. So I am familiar with the tag and I don't really like it, but I also think that standards are important and backwards compatibility is important.

I do wish that filterers learned to use whitelists of tags and attributes because the web will always evolve so making assumptions of what is the full dangerous set is never going to be safe for long.

(the clever regexp protection also fails to parse FORM and ForM btw. :-))

/Daniel

[Mike West]

On Mon, Dec 2, 2013 at 10:18 AM, Daniel Bratell <bra...@opera.com> wrote:

On Mon, 02 Dec 2013 10:03:09 +0100, Mike West <mk...@google.com> wrote:

Recent comments like https://twitter.com/0x6D6172696F/status/406006348577374208 bother me. :)

Unfortunately that one is protected, but I can imagine that <isindex> is mistreated somewhere on the web.

Ah, sorry. The link was a purely anecdotal appeal to authority: I'll respect the privacy setting, and paraphrase Mario's tweet as "<isindex> is more dangerous than you think."

That tag is included as an especially interesting tag in several of the fuzzers I've seen and before Presto used the current HTML5 parser it wasn't unheard of that <isindex> triggered some bug. So I am familiar with the tag and I don't really like it, but I also think that standards are important and backwards compatibility is important.

I completely agree that standards and compatibility are important.

I don't agree, however, that they should be the overriding concern. If data comes in that shows that the tag is as disused as I expect, then I would suggest that we weigh the potential of future bugs and unexpected behavior more highly than the risk to posterity.

 

I do wish that filterers learned to use whitelists of tags and attributes because the web will always evolve so making assumptions of what is the full dangerous set is never going to be safe for long.

Totally agree. Everyone should use a locked-down CSP on all their pages. That's unlikely to happen in the very near future, however.

 

(the clever regexp protection also fails to parse FORM and ForM btw. :-))

Well, yes. That's somewhat the point. There are already more than enough ways to do things wrong; I'd like to trim down some of the more esoteric paths. :)

-mike

[Anne van Kesteren]

Removing this in order to make blacklist-based filters perform better would set a dangerous precedent and limit our ability to introduce new features going forward.

Similar reasoning was used to argue against the new media elements and their corresponding event handler attributes. Not a good place to be.

--
http://annevankesteren.nl/

[Mike West]

Point taken.

That said, I think there's a relevant distinction between removing support for a disused and obsolete element with esoteric (but well-defined) behavior, and adding support for a new element or attribute.

-mike

--
Mike West <mk...@google.com>

Google+: https://mkw.st/+, Twitter: @mikewest, Cell: +49 162 10 255 91

Google Germany GmbH, Dienerstrasse 12, 80331 München, Germany
Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Graham Law, Christine Elizabeth Flores

[Anne van Kesteren]

On Mon, Dec 2, 2013 at 9:47 AM, Mike West <mk...@google.com> wrote:
> That said, I think there's a relevant distinction between removing support
> for a disused and obsolete element with esoteric (but well-defined)
> behavior, and adding support for a new element or attribute.

Agreed, but if you agree on giving in on the potential security
benefits the removal proposition becomes much weaker.


--
http://annevankesteren.nl/

[Mike West]

The point I attempted to make is that even theoretical security benefits can be given more weight when discussing the removal of an existing but obsolete and disused feature.

-mike

[Alex Russell]

On Mon, Dec 2, 2013 at 12:49 AM, Daniel Bratell <bra...@opera.com> wrote:

On Mon, 02 Dec 2013 09:22:37 +0100, Mike West <mk...@chromium.org> wrote:

Primary eng (and PM) emails

mk...@chromium.org

Summary

I would like to remove "support" for the obsolete isindex element[1] from Blink. I put support in quotes because what we currently do with the tag is strange, and doesn't seem to actually support it's usage in the wild.

Motivation

Currently, blink parses the isindex element by magically inserting a form and input element, along with some explanatory text (see HTMLTreeBuilder::processIsindexStartTagForInBody). This ends up looking like the "This is a searchable index. ..." at the top of [2], for example.

Typing something into that field and submitting the form reloads the page with the search query appended as a query string. In my testing, this has correctly searched for something on exactly 0 pages.

Given that the element is interpreted in a way that injects an unexpected form into a document, there is some non-trivial risk of a content injection attack making use of this element that probably isn't high on folks' lists of elements to filter[3]

This sounds like it's doing what it should do.

<isindex> is the predecessor of web forms and pretty ugly, and mostly just exists on historical sites from mid 90s. Those are not very many but they might still be of interest.

Even if it's in the obsolete section of the HTML specification, it's still in the HTML standard. I don't think it's setting a good precedent to go around dropping support for various parts of the HTML standard just because we think they are ugly.

I object to this line of reasoning. 

Many specs include "obsolete" or "conditionally normative" sections to denote how UAs must behave IF they implement uncommon features. These sections create no additional duty to support features, We should feel free to remove support to the extent that we think it won't break the web.

 

If I recall correctly the cost of supporting <isindex> is also very low since it just expands in the parser and no other part of the browser code needs to care about it.

/Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Daniel Bratell]

On Mon, 02 Dec 2013 21:00:06 +0100, Alex Russell <sligh...@google.com>
wrote:

But "break the web" is not a boolean. No browser supports 100% of the web,
and they are not even supporting the same subset of the web. To make it
possible for web developers to have a chance at writing pages and systems
we agree on standards, like the HTML standards. If a web developer keeps
to features described there is a fairly high chance that web browsers will
support that content now and in the future. It's like our promise to the
web developers.

I can understand that people weigh costs in the case of features like XSLT
(not one of the core standards like CSS, HTML, HTTP), with a bad
implementation in chromium, not that high usage and with a maintenance
cost that is noticeable by the people working on the code, but this is an
HTML feature, included in every HTML specification since the early days of
W3C, with a negligible maintenance cost.

In the case of the current HTML spec it's called obsolete, but that is for
content authors, not browser vendors (I know it's confusing, but you
should discuss that with Hixie and Anne, not here).

/Daniel

[Ojan Vafai]

I object to this line of reasoning.
Many specs include "obsolete" or "conditionally normative" sections to denote how UAs must behave >IF they implement uncommon features. These sections create no additional duty to support features, >We should feel free to remove support to the extent that we think it won't break the web.

But "break the web" is not a boolean. No browser supports 100% of the web, and they are not even supporting the same subset of the web. To make it possible for web developers to have a chance at writing pages and systems we agree on standards, like the HTML standards. If a web developer keeps to features described there is a fairly high chance that web browsers will support that content now and in the future. It's like our promise to the web developers.

I can understand that people weigh costs in the case of features like XSLT (not one of the core standards like CSS, HTML, HTTP), with a bad implementation in chromium, not that high usage and with a maintenance cost that is noticeable by the people working on the code, but this is an HTML feature, included in every HTML specification since the early days of W3C, with a negligible maintenance cost.

But, if we UseCounter it and find that it is pretty much unused, then the benefit to keeping it is also low. It's hard to measure to maintenance cost of a feature. In and of itself, it doesn't present much of a cost, but 100 such minor features do.

In either case, I don't think we should do anything with isindex without a UseCounter showing that the usage is very low.

[PhistucK]

Probably not a good idea, but you could implement it only within non HTML5 documents, or (and) disable its parser implementation when used within innerHTML, outerHTML and any other HTML parsing method (insertAdjacentHTML and such) and if document.createElement("isindex") also does magic, disable it.

I would guess that pages that use this element do not use JavaScript, or use something like DOM 0 (or even earlier attempts), since isindex seems to basically be an HTML 2 concept, from 1995. DOM level 1 (for example) was released in 1998, in which this was already a discouraged concept (HTML 4 was released in 1997).

☆PhistucK

[Alex Russell]

On Mon, Dec 2, 2013 at 12:44 PM, Ojan Vafai <oj...@chromium.org> wrote:

I object to this line of reasoning.
Many specs include "obsolete" or "conditionally normative" sections to denote how UAs must behave >IF they implement uncommon features. These sections create no additional duty to support features, >We should feel free to remove support to the extent that we think it won't break the web.

But "break the web" is not a boolean. No browser supports 100% of the web, and they are not even supporting the same subset of the web. To make it possible for web developers to have a chance at writing pages and systems we agree on standards, like the HTML standards. If a web developer keeps to features described there is a fairly high chance that web browsers will support that content now and in the future. It's like our promise to the web developers.

I can understand that people weigh costs in the case of features like XSLT (not one of the core standards like CSS, HTML, HTTP), with a bad implementation in chromium, not that high usage and with a maintenance cost that is noticeable by the people working on the code, but this is an HTML feature, included in every HTML specification since the early days of W3C, with a negligible maintenance cost.

But, if we UseCounter it and find that it is pretty much unused, then the benefit to keeping it is also low. It's hard to measure to maintenance cost of a feature. In and of itself, it doesn't present much of a cost, but 100 such minor features do.

Agreed about the UseCounter. I wasn't trying to make a judgement in my previous message about whether or not removing <isindex> would break anything as nobody (yet) has the data. But if we can convince ourselves that we won't break things, there's nothing about <isindex> being in the obsolete section of the HTML spec that argues for keeping it.

 

In either case, I don't think we should do anything with isindex without a UseCounter showing that the usage is very low.

+1.

[Yoshifumi Inoue]

Google index files say <isindex> is used 0.000011% of pages as of December 2, 2013.

-yosi

[Elliott Sprehn]

On Mon, Dec 2, 2013 at 9:17 PM, Yoshifumi Inoue <yo...@google.com> wrote:

Google index files say <isindex> is used 0.000011% of pages as of December 2, 2013.

-yosi

That's really really low. For that to be 0.1% of all pages viewed those would need to be very popular (amazon.com like) pages. I suppose there could be some libraries that are not indexed by Google using it we'd break though.

I support removing it. It's death by a thousand ancient features in the platform.

[Daniel Bratell]

On Tue, 03 Dec 2013 05:18:59 +0100, Elliott Sprehn <esp...@chromium.org>
wrote:

>

> On Mon, Dec 2, 2013 at 9:17 PM, Yoshifumi Inoue <yo...@google.com> wrote:
>
>
>> Google index files say <isindex> is used 0.000011% of pages as of
>> December 2, 2013.
>> -yosi
>>
>>
>>
>
> That's really really low. For that to be 0.1% of all pages viewed those
> would need to be very popular >(amazon.com like) pages. I suppose there
> could be some libraries that are not indexed by Google using >it we'd
> break though.
>
>
>
> I support removing it. It's death by a thousand ancient features in the
> platform.

Shouldn't the first step be to get it removed from the HTML specification?
Or why do we have standards?

/Daniel

[Anne van Kesteren]

On Tue, Dec 3, 2013 at 1:14 AM, Alex Russell <sligh...@google.com> wrote:
> Agreed about the UseCounter. I wasn't trying to make a judgement in my
> previous message about whether or not removing <isindex> would break
> anything as nobody (yet) has the data. But if we can convince ourselves that
> we won't break things, there's nothing about <isindex> being in the obsolete
> section of the HTML spec that argues for keeping it.

That is not how the specification works actually. The obsolete bit is
about conformance and aimed at authors. The normative bit aimed at
implementers is the parsing section (and the special case in form
submission, which can also be triggered independently of <isindex> so
you'd have to count that too somehow). Now the specification can be
changed and all other user agents could change too, but the question
is whether that churn is worthwhile for such a minor thing that does
no harm.


--
http://annevankesteren.nl/

[Mike West]

As an additional data point, I was just informed that Blink's XSS Auditor misses isindex-based vectors (which makes sense, as I think that the tag's magical behavior means that 'isindex' never appears in the token stream that the auditor sees (+tsepez: Hi!)).

If we can't get this right as a browser vendor, I don't imagine that developers will have much better luck. These are the sorts of things that make me question the claim that the tag "does no harm".

Thank you all for the discussion. I've added a usage counter in http://crrev.com/98743002; I'll circle back to this thread in a few weeks when I have more complete data to present.

-mike

-Mike

[Mike West]

On Tue, Dec 3, 2013 at 12:21 PM, Mike West <mk...@chromium.org> wrote:

As an additional data point, I was just informed that Blink's XSS Auditor misses isindex-based vectors (which makes sense, as I think that the tag's magical behavior means that 'isindex' never appears in the token stream that the auditor sees (+tsepez: Hi!)).

If we can't get this right as a browser vendor, I don't imagine that developers will have much better luck. These are the sorts of things that make me question the claim that the tag "does no harm".

Thank you all for the discussion. I've added a usage counter in http://crrev.com/98743002; I'll circle back to this thread in a few weeks when I have more complete data to present.

It's been a few weeks. M33 finally (finally!) hit stable. <isindex> is showing up in about 0.0049% of page loads.

I'd still like to remove it.

-mike

PS: It would be lovely if we could come up with clever ways of decreasing the mean time between landing a UseCounter in ToT, and getting useful data. ~12 weeks is a long time. :(

[PhistucK]

Regarding the PS, I agree. Perhaps setting up a new policy where use counters are acceptable merge candidates through the beta phase or even closer to stable.

☆PhistucK

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Eric Seidel]

Given the low usage numbers, lgtm.

[Daniel Bratell]

On Mon, 24 Feb 2014 17:13:31 +0100, Eric Seidel <ese...@chromium.org>
wrote:

> Given the low usage numbers, lgtm.

Making history.

<isindex> is one of the original ~20 tags in HTML. You can read about it
in http://www.w3.org/MarkUp/draft-ietf-iiir-html-01.txt which is the
earliest draft of HTML I could find.

The current HTML 5 specification is longer.

/Daniel

[Eric Seidel]

<isindex> already "died" from its historical form with the HTML5
parser with re-wrote it to inject a <form> tag and <submit> button
with a few other bits of markup. :) It was a clever hack for HTML5,
but glad to see it finally RIP.

[Daniel Bratell]

On Mon, 24 Feb 2014 18:17:39 +0100, Eric Seidel <ese...@chromium.org>
wrote:

> <isindex> already "died" from its historical form with the HTML5
> parser with re-wrote it to inject a <form> tag and <submit> button
> with a few other bits of markup. :) It was a clever hack for HTML5,
> but glad to see it finally RIP.

<isindex> predates scripting and the document object model so the change
from native support to parser-macro support was just an optimization,
following the classic rule that any optimization that can't be observed is
legal.

When you can no longer use pages from early/mid-90s (if any of those
servers are still alive) because of dropped client support, that is when
the tag is dead.

/Daniel

[Ian Hickson]

On Mon, 24 Feb 2014, Mike West wrote:
>
> It's been a few weeks. M33 finally (finally!) hit stable. <isindex> is
> showing up in about 0.0049% of page loads.
>
> I'd still like to remove it.

What exactly is it you want to remove? The parser entry for "isindex"
start tags? Parts of the form submission logic? Something else?

--
Ian Hickson U+1047E )\._.,--....,'``. fL
http://ln.hixie.ch/ U+263A /, _.. \ _\ ;`._ ,.
Things that are impossible just take longer. `._.-(,_..'--(,_..'`-.;.'

[Mike West]

On Mon, Feb 24, 2014 at 6:37 PM, Ian Hickson <i...@hixie.ch> wrote:

On Mon, 24 Feb 2014, Mike West wrote:
>
> It's been a few weeks. M33 finally (finally!) hit stable. <isindex> is
> showing up in about 0.0049% of page loads.
>
> I'd still like to remove it.

What exactly is it you want to remove? The parser entry for "isindex"
start tags? Parts of the form submission logic? Something else?

I'd like to remove the special-casing of <isindex> in the parser: https://codereview.chromium.org/96653004/ (which probably needs to be rebaselined).

This basically means that http://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cisindex%3E would end up behaving the same as http://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cfoo%3E with the exception of the node's name.

-mike

[Dimitri Glazkov]

LGTM2.

[Jochen Eisinger]

LGTM3

On Tue, Feb 25, 2014 at 9:32 PM, Dimitri Glazkov <dgla...@chromium.org> wrote:

LGTM2.

[marc....@gmail.com]

Please don't remove isindex.   It's really handy for simple lookup scripts; and I've got at least a dozen of them that have to

get changed if you guys keep breaking it.

http://cdcvs.fnal.gov/cgi-bin/searchregistered.cgi

http://cdcvs.fnal.gov/cgi-bin/searchaddproduct.cgi

etc. Were these scripts written back in the 1990's?  Yes.  Do they still work, yes, in browsers that still support

isindex. 

Please stop breaking old things just 'cause there are new ones.

On Monday, December 2, 2013 at 2:22:37 AM UTC-6, Mike West wrote:

Primary eng (and PM) emails

mk...@chromium.org

Summary

I would like to remove "support" for the obsolete isindex element[1] from Blink. I put support in quotes because what we currently do with the tag is strange, and doesn't seem to actually support it's usage in the wild.

Motivation

Currently, blink parses the isindex element by magically inserting a form and input element, along with some explanatory text (see HTMLTreeBuilder::processIsindexStartTagForInBody). This ends up looking like the "This is a searchable index. ..." at the top of [2], for example.

Typing something into that field and submitting the form reloads the page with the search query appended as a query string. In my testing, this has correctly searched for something on exactly 0 pages.

Given that the element is interpreted in a way that injects an unexpected form into a document, there is some non-trivial risk of a content injection attack making use of this element that probably isn't high on folks' lists of elements to filter[3]

[1]: http://www.whatwg.org/specs/web-apps/current-work/multipage/obsolete.html#isindex-0

[2]: http://www.ug.ru/old/95.25/250902.htm

[3]: http://www.thespanner.co.uk/2008/08/26/new-xss-vector/

[4]: http://www.w3.org/MarkUp/html-spec/html.dtd

Usage information from UseCounter

I don't have UseCounter data (and it's nontrivial to get, since the replacement happens somewhere where we don't currently have easy access to the counter mechanism). Grepping for the text through Google's search index reveals minimal usage: 756 results total on 341 unique domains (this includes pages like [4] that merely mention the tag; the tool's syntax is strange, and I don't know how to filter those out).

Compatibility Risk

Firefox supports isindex in exactly the same way that Blink does. Pages that rely on this behavior will break. So far I haven't found any.

Row on feature dashboard?

No.

WDYT?

-mike

[PhistucK]

Too little too late, it was removed two years ago...

☆PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.