bugGNU Wget - Bugs: bug #20653, Heuristic for determining non-HTML...

 
 

bug #20653: Heuristic for determining non-HTML files

Submitter:  Micah Cowan <micahcowan>
Submitted:  Wed 01 Aug 2007 06:38:25 PM UTC
   
 
Category:  Feature Request Severity:  3 - Normal
Priority:  9 - Immediate Status:  Confirmed
Privacy:  Public Assigned to:  derektdoyle
Originator Name:  Open/Closed:  Open
Release:  None Operating System:  None
Reproducibility:  None Fixed Release:  None
Planned Release:  1.14 Regression:  None
Work Required:  1 - Days Patch Included:  Yes
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 14 Dec 2008 10:26:30 PM UTC, comment #8: 

Here's a patch that does what I'm suggesting.

Thanks.


(file #17061)

 Derek T. Doyle <derektdoyle>
Sun 14 Dec 2008 12:35:03 AM UTC, comment #7: 

You're quite welcome.  I find Wget very useful both professionally and personally, so it's nice to be able to help out.  Thank you for all your hard work on it.

Good points all around.  I've added p, div, and span to the list along with table, form and li.  I corrected the strncpy issue by only reading just the length of fm->content if it's less than 1024, 1024 if it's greater or equal, and always terminate it by placing \0 one character beyond whatever the read length turned out to be.  Naturally, fk is now 1025 long instead of 1024, to make room for a possible \0 at that position.

It is now clear that c_tolower() isn't going to work the way I was using it.  As a result of your suggestion, I looked at strcasecmp(), but it only compares entire strings.  There doesn't seem to be a strcasestr() or stristr() in use anywhere to search for a substring case-insensitively.  In the spirit of your suggestion about insensitively matching DOCTYPE, I'm considering simply iterating through the length of the first 1k (or however much) and lower casing it all using c_tolower, then only comparing against the lowercase version of all the tags using strstr().  This seems like the least-code, permissive-but-not-too-permissive recognition scheme, letting in
"<hTml" and "<!DoctypE html" rare and out-of-spec though they may be.  It would also allow our struct to fall back to a simple array, discarding the case_sensitive flag. What would you think of this approach?

As far as "<p ", "<p>", and "<p/>", my instinct is to try each of these endings on all the tags in rec_tag_list as a simple rule-of-thumb with only a couple of unlikely cases in the bunch.

Thanks for your input on this.

Derek T. Doyle <derektdoyle>
Mon 01 Dec 2008 10:41:44 PM UTC, comment #6: 

Yeah, although now it'll also identify non-HTML SGML or XML applications as "HTML". But it is just a heuristic, after all.

I suspect that putting it back to "<!DOCTYPE HTML" and making it case-insensitive might produce mildly more-accurate results (even though "DOCTYPE" should never appear in lowercase).

Since we want this heuristic to err on the side of false positives rather than false negatives, it's probably worthwhile to add recognition for tags such as p, div, span, and possibly a few others, as folks have been known to omit head/body tags from HTML files. "<p" is probably too permissive, as it matches "<polyester"; but perhaps "<p ", "<p>", and "<p/>" are good.

As I read the code, that strncpy() will fail to result in a null-terminated string in the (likely) event that the length of the file's contents exceeds 1024. This makes it a bad thing to pass to strstr. (Also, if the content length is much less than 1024 (say, 80), strncpy() will still write a total of 1024 bytes.)

The use of c_tolower() to handle "case-insensitive" matching isn't going to work properly: it converts a single character, whereas you're passing a string. Bad things may happen. :)... anyway, it's not going to catch silly things like "<hTml"; better to use strcasecmp() instead.

BTW, thanks very much for helping with this!

 Micah Cowan <micahcowan>
Sun 30 Nov 2008 05:15:20 PM UTC, comment #5: 

Right.

I trimmed doctype check back to just "<!DOCTYPE" and made it case sensitive, restricted to upper case.

The loop over an array of specially-written structs sounded correct and elegant, so here it is in a patch that replaces the first one.

Thanks.


(file #16951)

 Derek T. Doyle <derektdoyle>
Mon 24 Nov 2008 02:58:35 AM UTC, comment #4: 

Of course, <!DOCTYPE HTML is also permissible (but not, as you know, <!doctype HTML).

Could probably implement the series of "if (tag_recognized" as a loop over an array of specially-written structs (string/case-sensitivity pair)?

 Micah Cowan <micahcowan>
Mon 24 Nov 2008 12:01:39 AM UTC, comment #3: 

OK.

I have submitted a patch that seems to work, in line with what I described in comment #2.

Currently it recognizes for DOCTYPE, html, head, and body (for the last three both upper and lower case).  Other tags could easily be added.

Thanks.


(file #16909)

 Derek T. Doyle <derektdoyle>
Sat 22 Nov 2008 04:25:51 PM UTC, comment #2: 

Hi,

This feature looks like an interesting one to add.  I have an approach in mind that I think would make this happen.  Could I get this feature assigned to me, please?

The place to make it happen seems to be in get_urls_html() in html-url.c, where fm->content is available to provide the first 1k of the file just prior to where map_html_tags() is called to do the parsing.  I figure that's where a simple check can be done for a few common tags.

If this heuristic would be useful anywhere else in the code, it could easily be made into a separate function.

Thanks.

 Derek T. Doyle <derektdoyle>
Wed 01 Aug 2007 06:39:37 PM UTC, comment #1: 

This request was filed in response to bug 20496.

 Micah Cowan <micahcowan>
Wed 01 Aug 2007 06:38:25 PM UTC, original submission:  

It could be useful for some heuristic mechanism to refuse to parse files that do not appear to be HTML. The mechanism would need to err on the safe, "is HTML" side, especially since many encodings could look a lot like binary data. Perhaps just checking for the existence of some recognized tag within the first 1k or so...

 Micah Cowan <micahcowan>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #17061:  wget_fix_for_bug_20653.3.patch added by derektdoyle (3KiB - text/x-patch)
file #16951:  wget_fix_for_bug_20653.2.patch added by derektdoyle (3KiB - text/x-patch - Here is the refinement as suggested by Micah, which replaces the previous patch.)
file #16909:  wget_fix_for_bug_20653.patch added by derektdoyle (3KiB - text/x-patch - Here is the first attempt, which seems to work.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by derektdoyle (Posted a comment)
  • -email is unavailable- added by derektdoyle (Looks interesting.)
  • -email is unavailable- added by micahcowan (Submitted the item)

    •  

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-12-14 derektdoyle Attached File- Added wget_fix_for_bug_20653.3.patch, #17061
    2008-12-01 micahcowan Priority4 9 - Immediate
        Patch IncludedNone Yes
    2008-11-30 derektdoyle Attached File- Added wget_fix_for_bug_20653.2.patch, #16951
    2008-11-24 micahcowan Planned Release1.15 1.14
    2008-11-24 derektdoyle Attached File- Added wget_fix_for_bug_20653.patch, #16909
    2008-11-22 micahcowan Assigned toNone derektdoyle
    2008-11-22 derektdoyle Carbon-Copy- Added -email is unavailable-
    2008-01-27 micahcowan Planned Release1.12 1.15
    2007-08-09 micahcowan Work RequiredNone 1 - Days
    2007-08-01 micahcowan Priority5 - Normal 4

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-3dd8.
    Corresponding source code