novoda / download-manager

A library that handles long-running downloads, handling the network interactions and retrying downloads automatically after failures
Apache License 2.0
483 stars 63 forks source link

Problem with URL containing query string #505

Open lkaminskivim opened 5 years ago

lkaminskivim commented 5 years ago

Hi,

When url contains query params (?name=val&...), then entire query string is included in file name. This is because FileNameExtractor.java keeps everything after last "/".

This leads FilePathCreator.java to create wrong path. This is because it eliminates redundant part of path using String.replaceAll method which treats its argument as regex. So in case of file name already containing query params (which contains special signs) it results in path like:

{intended_path_to_file}?{query_string}/{intended_path_to_file}?{query_string}

Mecharyry commented 5 years ago

Thank you very much for letting us know. We'll try and fix this as soon as we are able 👍

Mecharyry commented 5 years ago

Just an FYI that you can workaround this issue in the meantime by using https://github.com/novoda/download-manager/blob/release/library/src/main/java/com/novoda/downloadmanager/BatchFileBuilder.java#L35 to specify the path and file name directly for a file rather than using the extractor.

ouchadam commented 5 years ago

I would argue this isn't a responsibility of the library since there's a way to customise the creation of the filename 👻

Although it would be good to document the behaviour

jakubvimn commented 4 years ago

I think that it's the library responsibility to work correctly and the FileNameExtractor does not in all cases, because it can't handle properly urls with query parameters :) In order to fix the implementation, only one additional split is needed. Kotlin version below: assetUrl.split("/").last().split("?").first()