Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

163邮箱文档类型的附件下载失败,不能添加Range #921

Closed
congwiny opened this issue Jan 19, 2018 · 4 comments
Closed

163邮箱文档类型的附件下载失败,不能添加Range #921

congwiny opened this issue Jan 19, 2018 · 4 comments
Milestone

Comments

@congwiny
Copy link

经过我一番测试和log输出,确定原因是163邮箱不支持多线程断点下载,不能添加Range这个请求头。

我看了FileDownloader的源码:
1.执行下载任务时,会执行DownloadLauncherRunnable的run方法

 final ConnectTask.Builder build = new ConnectTask.Builder();
 final ConnectTask firstConnectionTask = build.setDownloadId(model.getId())
        .setUrl(model.getUrl())
        .setEtag(model.getETag())
        .setHeader(userRequestHeader)
         .setConnectionProfile(connectionProfile)
         .build();

  connection = firstConnectionTask.connect();
  handleFirstConnected(firstConnectionTask.getRequestHeader(), firstConnectionTask, connection);

上面这段代码是要和请求的资源建立第一次连接,看服务器是否支持206,如果支持就使用多线程断点下载。如果不支持就使用单线程下载文件。

在firstConnectionTask.connect();执行的是ConnectTask的connect方法。

 FileDownloadConnection connect() throws IOException, IllegalAccessException {
        FileDownloadConnection connection = CustomComponentHolder.getImpl().createConnection(url);

        addUserRequiredHeader(connection);
        addRangeHeader(connection);//每一次请求添加Range头

        // init request
        // get the request header in here, because of there are many connection
        // component(such as HttpsURLConnectionImpl, HttpURLConnectionImpl in okhttp3) don't
        // allow access to the request header after it connected.
        requestHeader = connection.getRequestHeaderFields();
        if (FileDownloadLog.NEED_LOG) {
            FileDownloadLog.d(this, "%s request header %s", downloadId, requestHeader);
        }

        connection.execute();
        redirectedUrlList = new ArrayList<>();
        connection = RedirectHandler.process(requestHeader, connection, redirectedUrlList);

        return connection;
    }

由上而知每次下载请求都会添加Range。第一次请求下载资源时是可以加的,判断服务器支不支持断点下载。如果确定服务器不支持断点下载,就不需要添加Range了吧。

在下载163邮箱文档类型的附件时,第一次请求下载失败。之前我为FileDownloader下载失败重试3次,所以接下来的第二次,第三次都会执行到ConnectTask中的addRangeHeader方法。导致下载任务失败。

后来,我把addRangeHeader方法给注释掉了。下载就成功了。但这样肯定是不行的,因为有些服务器的资源是支持多线程断点下载的。所以要做到兼容嘛。

以下是我的兼容方法,能正常下载163邮箱文档类型的附件,也能多线程断点下载其他资源。
还请 @Jacksgong review下,看看我的方法可行吗?还有没有更好的解决办法。

思路是在下载失败,重试的时候搞事情:
在DownloadLaunchRunnable添加一个成员变量isRangeNotSatisfiable,默认为false,在DownloadLaunchRunnable的isRetry方法里,修改isRangeNotSatisfiable的值。也为ConnectionProfile这个类添加一个成员变量isRangeNotSatisfiable,这个变量的取值根据DownloadLaunchRunnable中的isRangeNotSatisfiable取值。(在DownloadLaunchRunnable里创建ConnectionProfile时传入isRangeNotSatisfiable)

 @Override
    public boolean isRetry(Exception exception) {
        if (exception instanceof FileDownloadHttpException) {
            final FileDownloadHttpException httpException = (FileDownloadHttpException) exception;

            final int code = httpException.getCode();

            if (isSingleConnection && code == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) {
                if (!isTriedFixRangeNotSatisfiable) {
                    FileDownloadUtils.deleteTaskFiles(model.getTargetFilePath(), model.getTempFilePath());
                    isTriedFixRangeNotSatisfiable = true;
                    return true;
                }
            }
           //修改isRangeNotSatisfiable的值
            if (validRetryTimes > 0 && code == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE) {
                isRangeNotSatisfiable = true;
            }
        }

        return validRetryTimes > 0 && !(exception instanceof FileDownloadGiveUpRetryException);
    }

然后修改ConnectTask的addRangeHeader方法。

    void addRangeHeader(FileDownloadConnection connection) {
        if (connection.dispatchAddResumeOffset(etag, profile.startOffset)) {
            return;
        }

        if (!TextUtils.isEmpty(etag)) {
            connection.addHeader("If-Match", etag);
        }
        final String range;
        //根据profiler.isRangeNotSatisfiable判断是否需要添加Range头。
        if (!profile.isRangeNotSatisfiable) {
            if (profile.endOffset == 0) {
                range = FileDownloadUtils.formatString("bytes=%d-", profile.currentOffset);
            } else {
                range = FileDownloadUtils.formatString("bytes=%d-%d", profile.currentOffset, profile.endOffset);
            }
            connection.addHeader("Range", range);
        }
    }

以上就是我的修改点,如果有没考虑到的或者不规范的地方,还望指教哈~
还是希望得到更好的解决办法。

@rantianhua
Copy link
Collaborator

@congwiny 很赞,不过希望你能在 1.6.9 的版本上按照你的思路尽快提交一个 pull request ,到时候我们再详细 review ,ok 的话我们可以合入即将发布的 1.7.0 版本里。

@congwiny
Copy link
Author

@rantianhua 已经提交pull request啦 #925

@Jacksgong
Copy link
Collaborator

Jacksgong commented Jan 21, 2018

@congwiny 感谢你的PR,我Review了代码,但是这样修改将会引发其他正常任务出现新的BUG问题,原因如下:

  1. Range的作用不但是用于断点续传,还用于 分块下载,让每一块都是取回不同内容区间的输入流;因此这块影响的将会是断点续传,以及分块下载
  2. 416 错误是Range错误,意味着后端支持Range,但是本地给的Range是错误的Range,从协议层面来说,我们不应该粗暴将Range删除,而是想办法修复Range;目前的做法是在出现416的时候,会重新做一次请求0-的请求,获得完整的content-length,然后根据ConnectionAdapter的分块决策结果进行决定是否需要分块下载,如果需要分块下载将十分依赖Range;但是当第二次请求依然返回416将直接将错误信息返回给上层放弃下载
  3. 抛开上面提到的不说,单根据目前你的改动,在isRetry中如果发现是416,那么直接下一次重试中不添加Range,这将引入新的BUG --- 假设某一个分块写文件区间是[10,20),连接时的Range出现错误,重试时不添加Range,导致将[0, -)输入流的内容错误的写入[10,20)区间中,导致文件错乱

还是感谢你的PR,综上原因这个PR目前不会进行合入。


其实这个问题之前也有朋友提过,当时我已经给出现有版本的解决方案: #911 (comment) ,当时没有考虑修改库的原因,其实是目前不希望在FileDownloader中再引入新的Feature,我们需要将FileDownloader确定在一个稳定的版本,后续只做BUG FIX,而所有的新的Feature都将转移到更加稳定可靠、更加灵活的FileDownloader2- okdownload中进行处理。


但是还是多谢你的思考与Argue,给这个需求提供的一个相对可靠的思路。这边我们重新评估以后,预计通过以下方案来Cover这个问题:

首先根据你提供的信息,这个问题有一个很明显的特征,就是对于添加Range的任务,网易等做法是始终返回416错误,那么这将是一个很好的突破口,我们会根据这个特征采用以下策略解决这个问题:

  • 首先,其实我们目前的下载逻辑中,在每次启动一个任务下载,无论是分块下载,还是单连接下载,都会有一个 试探性的FristConnect 来根据后端的返回值来决定是否接下来通过分块进行下载
  • 其次,在首次 试探性的FirstConnect 中,如果返回是416,这时候判定当前次请求的Range是否是断点续传的Range,如果不是,那么判定即便是[0, -)后台判定也是错误的Range显然不成立,那么将保证下一次下载采用非分块下载,并且不再添加Range;如果是断点续传的 Range 那么按照原有逻辑去掉断点续传,从头进行 试探性的FristConnect,再进行判定

这么以来,对其他任务不会有不良影响,唯一的影响是,如果某一个任务使用一个绝对有效的Range依然返回416,那么该任务将自动不再带Range,并且不支持断点续传,不支持分块下载,自动转为从0开始从头单连接的从头开始下载。 该Feature1.7.0带上,并且会在okdownlaod中以可控策略的形式带上。

@Jacksgong Jacksgong added this to the 1.7.0 milestone Jan 21, 2018
@congwiny
Copy link
Author

非常感谢 @Jacksgong 专业细心的解答 ^_^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants