#16 Wrong Error Message (add_url is evil).

open
Morbus Iff
None
3
2003-12-12
2003-08-23
Chad Cloman
No

Version 0.93.1, Windows.

In the file Channels.pm, subroutine "load_channel", an
error in the call to XMLin() will result in the
creation of an error message that is placed in
$data->{item}->description. Prior to this point, $data
is empty. This situation occurs, for example, when an
un-escaped ampersand exists in the XML feed. So the
return value for a parsing error is a fake XML tree
containing the error message as an XML <item> tag.

Now transition to MyChannels.pm, subroutine "add_url",
where these lines of code are executed:
my $channel_data =
AmphetaDesk::Channels::load_channel($channel_raw_data);
unless ($channel_data) {
note("AmphetaDesk could not parse $url.", 1, 1);
next; # Entities, doctype's, and amperstands, oh my!
}
which prints the error message
"AmphetaDesk could not parse $url."
if $channel_data is returned as a zero value. By the
code comments, we can see that an un-escaped ampersand
is expected to trigger this portion of the code.

In the case of an un-escaped ampersand, a parsing error
occurred, but the return value was not zero; rather, it
was a "fake" XML tree with one item, as described
above. So the error check passes, the 'next' command is
not invoked, and the code continues to execute.

The error is caught further down in the routine with
this code:
else { # we have no clue, so spit an error and
return.
note("AmphetaDesk could not determine the
format of $url.", 1, 1); next;
}
which is meant to cover an XML input that does not
conform to the RSS/RDF format. The error message
printed here is misleading.

There are several different ways to fix this problem,
but I think the simplest would be to add a few lines of
code to the section of add_url that I listed earlier:
my $channel_data =
AmphetaDesk::Channels::load_channel($channel_raw_data);
unless ($channel_data) {
note("AmphetaDesk could not parse $url.", 1, 1);
next; # Entities, doctype's, and amperstands, oh my!
}
if (defined($channel_data->{item}) and not
defined($channel_data->{channel})) {
note("Error with
$url:\n$channel_data->{item}->{description}.", 1, 1);
next;
}

Discussion

  • Morbus Iff
    Morbus Iff
    2003-08-27

    • assigned_to: nobody --> morbus
     
  • Morbus Iff
    Morbus Iff
    2003-12-12

    Logged In: YES
    user_id=69804

    I'm low-prioritizing this one, as I've always hated add_url,
    and have been fiddling around with it in my head. Largely,
    the fake XML tree was a complete and utter hack that should
    never have been implemented. Likewise, add_url needs an
    overhaul to handle auto-discovery, saving into the OPML even
    though the feed has an error (so that it'll try again in the
    future), and so forth.

     
  • Morbus Iff
    Morbus Iff
    2003-12-12

    • priority: 5 --> 3
     
  • Morbus Iff
    Morbus Iff
    2003-12-12

    • summary: Wrong Error Message --> Wrong Error Message (add_url is evil).
     
  • Morbus Iff
    Morbus Iff
    2003-12-12

    Logged In: YES
    user_id=69804

    Likewise, another add_url evil is the hack for multiple URLs
    in a QUERY_STRING. Currently, we look around for ",http" and
    split on that, but that files on stuff like file://,
    https://, and blah blah blah. It's easy as pie to add those
    other entries, but I've got some nagging feeling that
    there's a much better way to do it.