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

fixed android 6.0+ fetch HEAD network error bugs #25

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 60 additions & 19 deletions lib/FileSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { Platform } from 'react-native';
import pathLib from 'path';
import RNFetchBlob from 'react-native-fetch-blob';
import RNFetchBlob from 'rn-fetch-blob';
import sha1 from 'crypto-js/sha1';
import URL from 'url-parse';

Expand Down Expand Up @@ -83,13 +83,16 @@ export class FileSystem {
* Android:
* Android appears to purge cache dir files so we should use document dir to play it safe (even with reactNativeFetchBlob abstraction) : https://developer.android.com/guide/topics/data/data-storage.html
*
* unified Android & ios path => CacheDir (2018-10-12)
*
* @returns {String} baseFilePath - base path that files are written to in local fs.
* @private
*/
_setBaseFilePath(fileDirName = null) {
let baseFilePath = (this.os == 'ios') ? RNFetchBlob.fs.dirs.CacheDir : RNFetchBlob.fs.dirs.DocumentDir;
return RNFetchBlob.fs.dirs.CacheDir + '/' + fileDirName + '/';
/*let baseFilePath = (this.os == 'ios') ? RNFetchBlob.fs.dirs.CacheDir : RNFetchBlob.fs.dirs.DocumentDir;
baseFilePath += '/' + fileDirName + '/';
return baseFilePath;
return baseFilePath;*/
}

/**
Expand Down Expand Up @@ -120,13 +123,27 @@ export class FileSystem {
/**
*
* Wrapper for https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#existspathstringpromise
*
* Add check File size 0 return false and remove cache file
* @param path - local relative file path.
* @returns {Promise} - boolean promise for if file exists at path or not.
*/
exists(path) {
this._validatePath(path);
return RNFetchBlob.fs.exists(pathLib.resolve(this.baseFilePath + path));
return new Promise(async (resolve, reject) => {
try {
let result = await RNFetchBlob.fs.exists(pathLib.resolve(this.baseFilePath + path));
if (result) {
let stat = await RNFetchBlob.fs.stat(this.baseFilePath + path);
if (stat && stat.size === 0) {
result = false;
await RNFetchBlob.fs.unlink(this.baseFilePath + path);
}
}
resolve(result);
} catch(err) {
reject(err);
}
});
}

/**
Expand All @@ -137,7 +154,7 @@ export class FileSystem {
* @throws error on invalid (non jpg, png, gif, bmp) url file type. NOTE file extension or content-type header does not guarantee file mime type. We are trusting that it is set correctly on the server side.
* @returns fileName {string} - A SHA1 filename that is unique to the resource located at passed in URL and includes an appropriate extension.
*/
async getFileNameFromUrl(url) {
async getFileNameFromUrl(url, headers) {

const urlParts = new URL(url);
const urlExt = urlParts.pathname.split('.').pop();
Expand All @@ -163,11 +180,14 @@ export class FileSystem {
extension = 'bmp';
break;
default:
extension = await this.getExtensionFromContentTypeHeader(url);
extension = await this.getExtensionFromContentTypeHeader(url, headers);
}

if (!extension) {
throw new Error('Unable to determine remote image filetype.');
//throw new Error('Unable to determine remote image filetype.');
// android fetch HEAD bug set extension is null links: https://github.com/facebook/react-native/issues/18440
// then return url to String
return sha1(url).toString();
}

return sha1(url).toString() + '.' + extension;
Expand All @@ -183,7 +203,7 @@ export class FileSystem {
* @param url {String} - An absolute url.
* @returns extension {string} - A file extension appropriate for remote file.
*/
async getExtensionFromContentTypeHeader(url) {
async getExtensionFromContentTypeHeader(url, headers = {}) {

let extension = null;
let contentType = null;
Expand All @@ -192,7 +212,8 @@ export class FileSystem {
try{

const response = await fetch(url, {
method: 'HEAD'
method: 'HEAD',
headers
});

if (response.headers.get('content-type')) {
Expand All @@ -212,6 +233,12 @@ export class FileSystem {
case 'image/gif':
extension = 'gif';
break;
case 'image/jpg':
extension = 'jpg';
break;
case 'image/heic':
extension = 'heic';
break;
case 'image/jpeg':
extension = 'jpg';
break;
Expand All @@ -233,13 +260,14 @@ export class FileSystem {
*
* @param url {String} - url of file to download.
* @param permanent {Boolean} - True persists the file locally indefinitely, false caches the file temporarily (until file is removed during cache pruning).
* @param headers {Object} request headers support
* @returns {Promise} promise that resolves to the local file path of downloaded url file.
*/
async getLocalFilePathFromUrl(url, permanent) {
async getLocalFilePathFromUrl(url, permanent, headers) {

let filePath = null;

let fileName = await this.getFileNameFromUrl(url);
let fileName = await this.getFileNameFromUrl(url, headers);

let permanentFileExists = this.exists('permanent/' + fileName);
let cacheFileExists = this.exists('cache/' + fileName);
Expand All @@ -256,9 +284,10 @@ export class FileSystem {

} else {

let result = await this.fetchFile(url, permanent, null, true); // Clobber must be true to allow concurrent CacheableImage components with same source url (ie: bullet point images).
filePath = result.path;

let result = await this.fetchFile(url, permanent, null, true, headers); // Clobber must be true to allow concurrent CacheableImage components with same source url (ie: bullet point images).
if (result) {
filePath = result.path;
}
}

return filePath;
Expand All @@ -275,9 +304,9 @@ export class FileSystem {
* @param clobber {String} - whether or not to overwrite a file that already exists at path. defaults to false.
* @returns {Promise} promise that resolves to an object that contains the local path of the downloaded file and the filename.
*/
async fetchFile(url, permanent = false, fileName = null, clobber = false) {
async fetchFile(url, permanent = false, fileName = null, clobber = false, headers = {}) {

fileName = fileName || await this.getFileNameFromUrl(url);
fileName = fileName || await this.getFileNameFromUrl(url, headers);
let path = this.baseFilePath + (permanent ? 'permanent' : 'cache') + '/' + fileName;
this._validatePath(path, true);

Expand All @@ -300,13 +329,25 @@ export class FileSystem {
.config({
path: path
})
.fetch('GET', url);
.fetch('GET', url, headers);

// File Content-Length:0 then remove on download
if (result && result.respInfo && result.respInfo.headers['Content-Length'] == '0') {
await RNFetchBlob.fs.unlink(path);
return null;
}

// File status not 200 then remove on download
if (result && result.respInfo && result.respInfo.status !== 200) {
await RNFetchBlob.fs.unlink(path);
return null;
}

} catch(error) {

// File must be manually removed on download error https://github.com/wkh237/react-native-fetch-blob/issues/331
await RNFetchBlob.fs.unlink(path);
throw error;
return null;

}

Expand Down
19 changes: 11 additions & 8 deletions lib/imageCacheHoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,14 @@ export default function imageCacheHoc(Image, options = {}) {

// Set url from source prop
const url = traverse(this.props).get(['source', 'uri']);
const headers = traverse(this.props).get(['source', 'headers']);

// Add a cache lock to file with this name (prevents concurrent <CacheableImage> components from pruning a file with this name from cache).
const fileName = await this.fileSystem.getFileNameFromUrl(url);
const fileName = await this.fileSystem.getFileNameFromUrl(url, headers);
FileSystem.lockCacheFile(fileName, this.componentId);

// Init the image cache logic
await this._loadImage(url);
await this._loadImage(url, headers);

}

Expand All @@ -168,20 +169,22 @@ export default function imageCacheHoc(Image, options = {}) {

// Set urls from source prop data
const url = traverse(this.props).get(['source', 'uri']);
const headers = traverse(this.props).get(['source', 'headers']);
const nextUrl = traverse(nextProps).get(['source', 'uri']);
const nextHeaders = traverse(nextProps).get(['source', 'headers']);

// Do nothing if url has not changed.
if (url === nextUrl) return;

// Remove component cache lock on old image file, and add cache lock to new image file.
const fileName = await this.fileSystem.getFileNameFromUrl(url);
const nextFileName = await this.fileSystem.getFileNameFromUrl(nextUrl);
const fileName = await this.fileSystem.getFileNameFromUrl(url, headers);
const nextFileName = await this.fileSystem.getFileNameFromUrl(nextUrl, headers);

FileSystem.unlockCacheFile(fileName, this.componentId);
FileSystem.lockCacheFile(nextFileName, this.componentId);

// Init the image cache logic
await this._loadImage(nextUrl);
await this._loadImage(nextUrl, nextHeaders);

}

Expand All @@ -193,13 +196,13 @@ export default function imageCacheHoc(Image, options = {}) {
* @param url {String} - The remote image url.
* @private
*/
async _loadImage(url) {
async _loadImage(url, headers) {

// Check local fs for file, fallback to network and write file to disk if local file not found.
const permanent = this.props.permanent ? true : false;
let localFilePath = null;
try {
localFilePath = await this.fileSystem.getLocalFilePathFromUrl(url, permanent);
localFilePath = await this.fileSystem.getLocalFilePathFromUrl(url, permanent, headers);
} catch (error) {
console.warn(error); // eslint-disable-line no-console
}
Expand All @@ -219,7 +222,7 @@ export default function imageCacheHoc(Image, options = {}) {
this._isMounted = false;

// Remove component cache lock on associated image file on component teardown.
let fileName = await this.fileSystem.getFileNameFromUrl(traverse(this.props).get(['source', 'uri']));
let fileName = await this.fileSystem.getFileNameFromUrl(traverse(this.props).get(['source', 'uri']), traverse(this.props).get(['source', 'headers']));
FileSystem.unlockCacheFile(fileName, this.componentId);

}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
"crypto-js": "^3.1.9-1",
"path": "^0.12.7",
"prop-types": "^15.6.0",
"react-native-fetch-blob": "0.10.8",
"react-native-uuid": "^1.4.9",
"rn-fetch-blob": "^0.10.11",
"traverse": "^0.6.6",
"url-parse": "^1.2.0",
"validator": "^9.0.0"
Expand Down