bugGNU Wget - Bugs: bug #29279, --conver-links and...

 
 

bug #29279: --conver-links and background-image: url() produces wrong output

Submitter:  Fernando Gutiérrez <zeg>
Submitted:  Sun 21 Mar 2010 03:37:09 AM UTC
   
 
Category:  Program Logic Severity:  3 - Normal
Priority:  5 - Normal Status:  None
Privacy:  Public Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  1.12 Operating System:  GNU/Linux
Reproducibility:  Every Time Fixed Release:  None
Planned Release:  None Regression:  None
Work Required:  None Patch Included:  No
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 24 Mar 2010 08:00:36 PM UTC, comment #5: 

The original submission is mistaken, I didn't understand the bug until I debugged it, so the title is misleading. Do not confuse with #29322, it is a different bug.

Proper explanation:
The problem only happens when a tag has a style attribute, this attribute is not the last one and the style has any urls (that's why it works with absolute urls, they are not modified) to be converted with --convert-links.

So, <a style="background-image: url('foo.gif');" href="somepage.htm">text</a> is bugged

and <a href="somepage.htm" style="background-image: url('foo.gif');">text</a> is not

Fernando Gutiérrez <zeg>
Wed 24 Mar 2010 10:07:58 AM UTC, comment #4: 

The last post was a messy brainstorming, the fix implemented is:

-ALL the handlers of tags that support style, MUST check for it.
-For tags without a handler, we check for a style attribute as usual.

Fernando Gutiérrez <zeg>
Wed 24 Mar 2010 10:01:49 AM UTC, comment #3: 

Problem:

Tag handlers add urlpos structures to the tail of urlpos list (tail pointer in ctx structure). That is done when they call append_url.

After the handler is called, check_style_attr checks for a style attribute in all the tags, and if found, more urlpos structures are added to the tail of the list.

If that happens, and any urlpos structures had been added in its handler, the list isn't ordered any more. That is a problem, it must be ordered when writing the output file if you are using convert-links.

Fix:

You can't really guess where the urlpos structures found in the style attribute should be inserted in the list referenced by pointers head and tail in ctx structure.

Not without parsing all the tags again, and that's already been done by their handlers. check_style_attr is important because if you remove it, style would be ignored for tags not in known_tags[] (well, not in interesting_tags), like div.

Normal tag handlers must be changed so they check for a style attribute if the tag allows it. Then you could somehow remember the position in the list and pass it to check_attributes, but that would be a mess. As you must find the style position, We can just call get_urls, the same thing check_style_attr is doing.
This way urls in style attributes are inserted in order.

Code is needed to avoid calling check_style_attributes in those cases where the handler already does the work. An easy but hackish way of doing it is storing the ctx->tail pointer before calling the handler. Because handlers only add
urls to the tail, if it doesn't change, then no urls have been added, and this bug wouldn't be a problem. This is easy to implement, but if you change the way handlers work, if they inserted urls at random positions instead of appending them to the list, THIS CODE WOULD BREAK. So I'm not doing it.

Another fix would be forcing all the handlers to check for a style attribute. Then you would call check_style_attributes just for those tags not in interesting_tags (the ones that don't have a handler). The only handlers that could avoid checking for style would be the ones only used by tags that support the attribute AND DON'T add URLs to the list. There are no such handlers (yet).

Only meta and base handlers are not checking for this, because they don't support style. All the others do, and also add URLs. I'm explaining this because I was going to implement an array to add tags you want to check for style and don't want to do it in the handler. However, it adds complexity and wouldn't be used much, certainly not now. So again, I didn't do it.

So, ALL the handlers of tags that support style, MUST check for it. No exception.


An alternative to avoid changing the handlers would be to store the number of attributes with URLs a tag has. Then check_style_attr could find the style attribute and know where it should be inserted, counting the number of those attributes in all the previous tags. Once known that it's just a matter of finding the position in the urlpos linked list.

In my opinion, this is a lot more inefficient and complicated, I see no easy fix to avoid forcing you to do the check in the handler. Anyway I don't think this is a problem. I tried to avoid touching the handlers just because I wanted to fix it without changing too many things.


To test the patch: wget --convert-links http://pastehtml.com/view/5txt9l4.html

Fernando Gutiérrez <zeg>
Tue 23 Mar 2010 02:46:29 PM UTC, comment #2: 

I have a patch, I will upload it and a technical explanation of the bug in a few hours.

Fernando Gutiérrez <zeg>
Sun 21 Mar 2010 03:50:10 AM UTC, comment #1: 

I found this while trying to download a mediawiki page with the Monobook skin. Wikipedia also uses that skin, but it works because they are using a full url to point to the background. However it can be changed in the config file, giving you that problem with wget. Anyway, syntax is valid, so many webpages will be using it, not just mediawiki.

Fernando Gutiérrez <zeg>
Sun 21 Mar 2010 03:37:09 AM UTC, original submission:  

Wget 1.12

When you use --conver-links with a webpage that has style="background-image: url('foo.gif');" , the produced webpage has additional trash code attached. This only happens when you don't use an absolute path for the url, for example url('http://example.com/foo.gif') would work as expected.


***Given the following webpage:
http://pastehtml.com/view/5tw41w3.html

<html>
  <head>
    <title>Testing</title>
  </head>
  <body>
    <a style="background-image: url('foo.gif');" href="somepage.htm">text</a>
  </body>
</html>

***Executing:
wget --convert-links -d http://pastehtml.com/view/5tw41w3.html

***we get:

<html>
  <head>
    <title>Testing</title>
  </head>
  <body>
    <a style="background-image: url('foo.gif');" href="http://pastehtml.com/view/somepage.htm">text</a>
  </body>
</html>

http://pastehtml.com/view/foo.gif');" href="somepage.htm">text</a>
  </body>
</html>

Fernando Gutiérrez <zeg>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #20013:  html-url.c.patch added by zeg (9KiB - application/octet-stream - Patch for version 1.12)
file #19991:  log.txt added by zeg (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by zeg (Submitted the item)
  • -email is unavailable- added by zeg
  •  

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-03-24 zeg Attached File- Added html-url.c.patch, #20013
    2010-03-21 zeg Attached File- Added log.txt, #19991
        Carbon-Copy- Added zeg

    Back to the top

    Powered by Savane 3.14-f13d.
    Corresponding source code