Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??

harangzsolt33:

In your first section, you tell the user 'Argument missing.' when they provide too many arguments. That's misleading, I'd change that to something like "Please provide only one URL" or similar.

In your Download() function, I see a couple things:

  • You're constructing a wget command and then running it, but causing yourself a bit of grief here: Using the backticks (or qx) executes the command as a shell command, meaning that you have to take extra care quoting the arguments--otherwise the shell may make a hash of things for you.
    Even worse is that you're opening yourself up to unexpected command injection, possibly giving a malicious user an opportunity to break into your machine. You can avoid both of these problems by using the system() function to execute a command on your system. By giving it the command and arguments as a list, like this:
    my $err_code = system 'wget', '-q', -O', $FILENAME, $URL; if ($err_code) { if ($err_code == 1) { EXIT(-1, "wget: Generic error code") } elsif ($err_code==2) { EXIT(-1, "wget: Parse error (cmdline, .wget +rc, ...)") } elsif ($err_code==3) { EXIT(-1, "wget: File I/O error") } ... etc. ... }
    you can avoid the quoting difficulties and presenting the shell as an attack surface.
  • Why are you creating a directory on your local machine? Just download the file to a temporary location, send it to the console and delete it. There's no reason you need to create directories in various locations. Additionally, since you're not cleaning the directories up when you're done with them, malicious users could start creating hundreds of thousands of directories on your machine, which can cause your machine to get very slow or even run out of disk space.
  • Speaking of downloading to a temporary location, you can use File::Temp to generate temporary file handles for you, so you don't accidentally make a collision. Your RandomString(8) is unlikely to collide, but File::Temp is a much-used (and tested!) module that will avoid running into files that already exist.

The toJStr() function is a bit suspicious, as it looks homegrown--like a "simple" task where you keep adding in exceptions as you find new ones. What happens if you get a binary string containing UTF-8 characters not in the ASCII range? Should your string be encoded?

One interesting thing might be what happens when you pass it something like "\00765". I didn't test it, but I'm thinking it's not going to end well.

The escape() and unescape() functions look unnecessarily opaque, and I worry that they may be homegrown things that don't have all the special cases handled. If they're "tried and true", you may want to mention that in a comment, as well as where they came from so you can treat them like "black boxes". That way, if some error occurs in the future, you can decide to dig into them or not based on your comment(s).

Also, why do you hate whitespace? If they were formatted better, at least you could look them over and get a feel for what they do.

Your Ceil() and Floor() functions are straightforward enough, that being one-liners seems OK. Even so, I'd still put a little whitespace in there for readability.

...roboticus

When your only tool is a hammer, all problems look like your thumb.


In reply to Re: wget not working from perl by roboticus
in thread wget not working from perl by harangzsolt33

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others cooling their heels in the Monastery: (5)
As of 2024-04-19 22:46 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found