已合并
fix: 修复下载pdf时getFilePath非法路径的处理;对@ohos.request的request.downloadFile不支持大写scheme(HTTP://)URL的情况，做规范化处理 #34
mazheng创建于 9月1日
fix: 修复下载pdf时getFilePath非法路径的处理;对@ohos.request的request.downloadFile不支持大写scheme(HTTP://)URL的情况，做规范化处理 #34
已合并
共 8 个文件变更+94-53
| @@ -1,5 +1,14 @@ | |||
| 1 | # Changelog | 1 | # Changelog |
| 2 | 2 | ||
| 3 | +## v2.8.3-beta.1 (2026-09-03) | ||
| 4 | + | ||
| 5 | +- Fix: await removeFile before downloadFile to avoid download race condition [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 6 | +- Fix: handle invalid fileName in getFilePath and normalize URL scheme [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 7 | +- Fix: remove unreachable try/catch dead code in onDownloadComplete, shareFile handles error catching uniformly [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 8 | +- Fix: capture error code in downloadTask fail callback and pass it to JS side [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 9 | +- Fix: replace Unicode multiplication sign with ASCII x in jpeg test URL [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 10 | +- Refactor: reduce download() nesting depth and fix codecheck issues [#34](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/34) | ||
| 11 | + | ||
| 3 | ## v2.8.2-rc.1 (2026-08-29) | 12 | ## v2.8.2-rc.1 (2026-08-29) |
| 4 | 13 | ||
| 5 | - docs:modify document scanning issues [#31](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/31) | 14 | - docs:modify document scanning issues [#31](https://gitcode.com/CPF-RN/rntpc_react-native-doc-viewer/pull/31) |
| @@ -8,13 +8,13 @@ | |||
| 8 | "start": "hdc rport tcp:8081 tcp:8081 && react-native start", | 8 | "start": "hdc rport tcp:8081 tcp:8081 && react-native start", |
| 9 | "reStart": "npm run install:pkg && npm run codegen && hdc rport tcp:8081 tcp:8081 && react-native start", | 9 | "reStart": "npm run install:pkg && npm run codegen && hdc rport tcp:8081 tcp:8081 && react-native start", |
| 10 | "pack:pkg": "cd ../ && npm pack && cd ./example", | 10 | "pack:pkg": "cd ../ && npm pack && cd ./example", |
| 11 | - "install:pkg": "npm uninstall @react-native-ohos/react-native-doc-viewer && npm run pack:pkg && npm i @react-native-ohos/react-native-doc-viewer@file:../react-native-ohos-react-native-doc-viewer-2.8.2-rc.1.tgz", | 11 | + "install:pkg": "npm uninstall @react-native-ohos/react-native-doc-viewer && npm run pack:pkg && npm i @react-native-ohos/react-native-doc-viewer@file:../react-native-ohos-react-native-doc-viewer-2.8.3-beta.1.tgz", |
| 12 | "dev": "npm run codegen && react-native bundle-harmony --dev --minify=false", | 12 | "dev": "npm run codegen && react-native bundle-harmony --dev --minify=false", |
| 13 | "postinstall": "node ./scripts/create-build-profile", | 13 | "postinstall": "node ./scripts/create-build-profile", |
| 14 | "codegen": "react-native codegen-harmony --rnoh-module-path ./harmony/entry/oh_modules/@rnoh/react-native-openharmony" | 14 | "codegen": "react-native codegen-harmony --rnoh-module-path ./harmony/entry/oh_modules/@rnoh/react-native-openharmony" |
| 15 | }, | 15 | }, |
| 16 | "dependencies": { | 16 | "dependencies": { |
| 17 | - "@react-native-ohos/react-native-doc-viewer": "file:../react-native-ohos-react-native-doc-viewer-2.8.2-rc.1.tgz", | 17 | + "@react-native-ohos/react-native-doc-viewer": "file:../react-native-ohos-react-native-doc-viewer-2.8.3-beta.1.tgz", |
| 18 | "@rnoh/testerino": "npm:@react-native-oh-tpl/testerino@0.0.9", | 18 | "@rnoh/testerino": "npm:@react-native-oh-tpl/testerino@0.0.9", |
| 19 | "react": "19.1.1", | 19 | "react": "19.1.1", |
| 20 | "react-native": "0.82.1", | 20 | "react-native": "0.82.1", |
| @@ -30,7 +30,7 @@ export function DocViewerExample() { | |||
| 30 | mp3: 'https://filesamples.com/samples/audio/mp3/sample3.mp3', | 30 | mp3: 'https://filesamples.com/samples/audio/mp3/sample3.mp3', |
| 31 | mp4: 'https://filesamples.com/samples/video/mp4/sample_960x400_ocean_with_audio.mp4', | 31 | mp4: 'https://filesamples.com/samples/video/mp4/sample_960x400_ocean_with_audio.mp4', |
| 32 | doc: 'https://filesamples.com/samples/document/doc/sample1.doc', | 32 | doc: 'https://filesamples.com/samples/document/doc/sample1.doc', |
| 33 | - jpeg: 'https://filesamples.com/samples/image/jpeg/sample_1280×853.jpeg', | 33 | + jpeg: 'https://filesamples.com/samples/image/jpeg/sample_1280x853.jpeg', |
| 34 | xlsx: 'https://filesamples.com/samples/document/xlsx/sample1.xlsx', | 34 | xlsx: 'https://filesamples.com/samples/document/xlsx/sample1.xlsx', |
| 35 | xml: 'https://gitee.com/apache/commons-digester/blob/master/commons-digester3-examples/pom.xml' | 35 | xml: 'https://gitee.com/apache/commons-digester/blob/master/commons-digester3-examples/pom.xml' |
| 36 | } | 36 | } |
| @@ -23,7 +23,7 @@ | |||
| 23 | */ | 23 | */ |
| 24 | 24 | ||
| 25 | export default class BuildProfile { | 25 | export default class BuildProfile { |
| 26 | - static readonly HAR_VERSION = '2.8.2-rc.1'; | 26 | + static readonly HAR_VERSION = '2.8.3-beta.1'; |
| 27 | static readonly BUILD_MODE_NAME = 'debug'; | 27 | static readonly BUILD_MODE_NAME = 'debug'; |
| 28 | static readonly DEBUG = true; | 28 | static readonly DEBUG = true; |
| 29 | static readonly TARGET_NAME = 'default'; | 29 | static readonly TARGET_NAME = 'default'; |
| @@ -1,6 +1,6 @@ | |||
| 1 | { | 1 | { |
| 2 | "name": "@react-native-ohos/react-native-doc-viewer", | 2 | "name": "@react-native-ohos/react-native-doc-viewer", |
| 3 | - "version": "2.8.2-rc.1", | 3 | + "version": "2.8.3-beta.1", |
| 4 | type: 'module', | 4 | type: 'module', |
| 5 | dependencies: { | 5 | dependencies: { |
| 6 | "@rnoh/react-native-openharmony": 'file:../react_native_openharmony' | 6 | "@rnoh/react-native-openharmony": 'file:../react_native_openharmony' |
| @@ -98,6 +98,6 @@ const mimeMap = { | |||
| 98 | } | 98 | } |
| 99 | 99 | ||
| 100 | export function getMimeType(fileType: string) { | 100 | export function getMimeType(fileType: string) { |
| 101 | - fileType = fileType.toLowerCase() | 101 | + fileType = typeof fileType === 'string' ? fileType.toLowerCase() : '' |
| 102 | return mimeMap[fileType] || mimeMap['pdf'] | 102 | return mimeMap[fileType] || mimeMap['pdf'] |
| 103 | } | 103 | } |
| @@ -69,6 +69,10 @@ export class DocViewTurboModule extends TurboModule{ | |||
| 69 | if (base64 && fileName && fileType) { | 69 | if (base64 && fileName && fileType) { |
| 70 | try{ | 70 | try{ |
| 71 | const filePath = this.getFilePath(fileName) | 71 | const filePath = this.getFilePath(fileName) |
| 72 | + if (!filePath) { | ||
| 73 | + callback(`invalid fileName:${fileName}`) | ||
| 74 | + return | ||
| 75 | + } | ||
| 72 | if (cache) { | 76 | if (cache) { |
| 73 | Log.debug(`try to use cache base64 file`) | 77 | Log.debug(`try to use cache base64 file`) |
| 74 | this.useCache(fileType, '', fileName, callback, async () => { | 78 | this.useCache(fileType, '', fileName, callback, async () => { |
| @@ -92,14 +96,16 @@ export class DocViewTurboModule extends TurboModule{ | |||
| 92 | const { url, fileName, fileType, cache } = fileParams[0] | 96 | const { url, fileName, fileType, cache } = fileParams[0] |
| 93 | try{ | 97 | try{ |
| 94 | if (url) { | 98 | if (url) { |
| 99 | + // scheme 大小写归一化，避免 FILE:// / HTTP:// 等写法无法被识别 | ||
| 100 | + const docUrl = this.normalizeUrl(url) | ||
| 95 | // 检查是否为本地文件（file:// 协议） | 101 | // 检查是否为本地文件（file:// 协议） |
| 96 | - if (url.startsWith('file://')) { | 102 | + if (docUrl.startsWith('file://')) { |
| 97 | - await this.handleLocalFile(url, fileName, fileType, callback) | 103 | + await this.handleLocalFile(docUrl, fileName, fileType, callback) |
| 98 | } else { | 104 | } else { |
| 99 | if (cache) { | 105 | if (cache) { |
| 100 | - this.useCache(fileType, url, fileName, callback) | 106 | + this.useCache(fileType, docUrl, fileName, callback) |
| 101 | } else { | 107 | } else { |
| 102 | - await this.download(url, fileType, fileName, callback) | 108 | + await this.download(docUrl, fileType, fileName, callback) |
| 103 | } | 109 | } |
| 104 | } | 110 | } |
| 105 | } else { | 111 | } else { |
| @@ -114,14 +120,16 @@ export class DocViewTurboModule extends TurboModule{ | |||
| 114 | const { url, fileName, fileType, cache } = fileParams[0] | 120 | const { url, fileName, fileType, cache } = fileParams[0] |
| 115 | try{ | 121 | try{ |
| 116 | if (url) { | 122 | if (url) { |
| 123 | + // scheme 大小写归一化，避免 FILE:// / HTTP:// 等写法无法被识别 | ||
| 124 | + const docUrl = this.normalizeUrl(url) | ||
| 117 | // 检查是否为本地文件（file:// 协议） | 125 | // 检查是否为本地文件（file:// 协议） |
| 118 | - if (url.startsWith('file://')) { | 126 | + if (docUrl.startsWith('file://')) { |
| 119 | - await this.handleLocalFile(url, fileName, fileType, callback) | 127 | + await this.handleLocalFile(docUrl, fileName, fileType, callback) |
| 120 | } else { | 128 | } else { |
| 121 | if (cache) { | 129 | if (cache) { |
| 122 | - this.useCache(fileType, url, fileName, callback) | 130 | + this.useCache(fileType, docUrl, fileName, callback) |
| 123 | } else { | 131 | } else { |
| 124 | - await this.download(url, fileType, fileName, callback) | 132 | + await this.download(docUrl, fileType, fileName, callback) |
| 125 | } | 133 | } |
| 126 | } | 134 | } |
| 127 | } else { | 135 | } else { |
| @@ -132,23 +140,27 @@ export class DocViewTurboModule extends TurboModule{ | |||
| 132 | } | 140 | } |
| 133 | } | 141 | } |
| 134 | getFilePath(fileName: string, url?: string) { | 142 | getFilePath(fileName: string, url?: string) { |
| 135 | - const context = this.ctx.uiAbilityContext | 143 | + let name = fileName ?? '' |
| 136 | - let filedDir = this.tempDir | 144 | + if (!name && url) { |
| 137 | - if (fileName) { | 145 | + name = url.split('/').pop() ?? '' |
| 138 | - return `${filedDir}/${fileName}` | ||
| 139 | } | 146 | } |
| 140 | - if (url) { | 147 | + name = name.split('?')[0].split('#')[0] |
| 141 | - const urlSplit = url?.split('/') | 148 | + name = name.split('/').pop() ?? '' |
| 142 | - const name = urlSplit[urlSplit.length - 1] | 149 | + name = name.split('\\').pop() ?? '' |
| 143 | - Log.debug(`getFilePath name:${name}`) | 150 | + if (!name || name === '.' || name === '..') { |
| 144 | - const filePath = filedDir + `/${name}` | 151 | + Log.debug(`getFilePath invalid fileName:${JSON.stringify(fileName)}, url:${url}`) |
| 145 | - return filePath | 152 | + return '' |
| 146 | } | 153 | } |
| 147 | - return '' | 154 | + Log.debug(`getFilePath name:${name}`) |
| 155 | + return `${this.tempDir}/${name}` | ||
| 148 | } | 156 | } |
| 149 | async useCache(fileType: string, url: string, fileName: string, callback: Function, notExistsFn?: Function) { | 157 | async useCache(fileType: string, url: string, fileName: string, callback: Function, notExistsFn?: Function) { |
| 150 | Log.debug(`useCache start`) | 158 | Log.debug(`useCache start`) |
| 151 | const filePath = this.getFilePath(fileName, url) | 159 | const filePath = this.getFilePath(fileName, url) |
| 160 | + if (!filePath) { | ||
| 161 | + callback(`invalid fileName:${fileName}`) | ||
| 162 | + return | ||
| 163 | + } | ||
| 152 | const isExists = await fs.access(filePath) | 164 | const isExists = await fs.access(filePath) |
| 153 | if (isExists) { | 165 | if (isExists) { |
| 154 | Log.debug(`useCache isExists:${filePath}`) | 166 | Log.debug(`useCache isExists:${filePath}`) |
| @@ -224,43 +236,63 @@ export class DocViewTurboModule extends TurboModule{ | |||
| 224 | } | 236 | } |
| 225 | } | 237 | } |
| 226 | 238 | ||
| 239 | + normalizeUrl(url: string): string { | ||
| 240 | + const idx = url.indexOf('://'); | ||
| 241 | + if (idx > 0) { | ||
| 242 | + return `${url.substring(0, idx).toLowerCase()}://${url.substring(idx + 3)}`; | ||
| 243 | + } | ||
| 244 | + return url; | ||
| 245 | + } | ||
| 246 | + | ||
| 227 | async download(url: string, fileType: string, fileName: string, callback: Function) { | 247 | async download(url: string, fileType: string, fileName: string, callback: Function) { |
| 228 | - Log.debug(`download start url:${url}`) | 248 | + url = this.normalizeUrl(url); |
| 229 | - const filePath = this.getFilePath(fileName, url) | 249 | + Log.debug(`download start url:${url}`); |
| 230 | - this.removeFile(filePath) | 250 | + const filePath = this.getFilePath(fileName, url); |
| 231 | - const context = this.ctx.uiAbilityContext | 251 | + if (!filePath) { |
| 232 | - try{ | 252 | + callback(`invalid fileName:${fileName}`); |
| 233 | - request.downloadFile(context, { | 253 | + return; |
| 254 | + } | ||
| 255 | + await this.removeFile(filePath); | ||
| 256 | + const context = this.ctx.uiAbilityContext; | ||
| 257 | + try { | ||
| 258 | + const downloadTask = await request.downloadFile(context, { | ||
| 234 | url, | 259 | url, |
| 235 | filePath | 260 | filePath |
| 236 | - }).then(downloadTask => { | 261 | + }); |
| 237 | - Log.debug(`downloadTask start`) | 262 | + Log.debug(`downloadTask start`); |
| 238 | - downloadTask.on('complete', () => { | 263 | + this.watchDownloadTask(downloadTask, filePath, fileType, fileName, callback); |
| 239 | - Log.debug(`download complete:${fileName}`) | ||
| 240 | - this.shareFile(filePath, fileType, callback) | ||
| 241 | - }) | ||
| 242 | - downloadTask.on('fail', () => { | ||
| 243 | - Log.debug(`download fail:${fileName}`) | ||
| 244 | - this.removeFile(filePath) | ||
| 245 | - callback(`download fail`) | ||
| 246 | - }) | ||
| 247 | - }).catch(err => { | ||
| 248 | - Log.debug(`Invoke catch downloadTask failed:${JSON.stringify(err)}`) | ||
| 249 | - }) | ||
| 250 | } catch (err) { | 264 | } catch (err) { |
| 251 | - Log.debug(`Invoke catch downloadTask failed:${JSON.stringify(err)}`) | 265 | + Log.debug(`Invoke catch downloadTask failed:${JSON.stringify(err)}`); |
| 252 | if (err.code === 13400002) { | 266 | if (err.code === 13400002) { |
| 253 | - Log.debug(`file is exists:${fileName}`) | 267 | + Log.debug(`file is exists:${fileName}`); |
| 254 | - this.shareFile(filePath, fileType, callback) | 268 | + this.shareFile(filePath, fileType, callback); |
| 255 | } else { | 269 | } else { |
| 256 | - callback(`download fail`) | 270 | + callback(`download fail`); |
| 257 | } | 271 | } |
| 258 | } | 272 | } |
| 259 | } | 273 | } |
| 274 | + | ||
| 275 | + watchDownloadTask(downloadTask: request.DownloadTask, filePath: string, fileType: string, fileName: string, callback: Function) { | ||
| 276 | + downloadTask.on('complete', () => { | ||
| 277 | + Log.debug(`download complete:${fileName}`); | ||
| 278 | + this.shareFile(filePath, fileType, callback); | ||
| 279 | + }); | ||
| 280 | + downloadTask.on('fail', (err: number) => { | ||
| 281 | + Log.debug(`download fail:${fileName}, code:${err}`); | ||
| 282 | + this.removeFile(filePath); | ||
 | |||
| 283 | + callback(`download fail, code:${err}`); | ||
| 284 | + }); | ||
| 285 | + } | ||
| 286 | + | ||
| 260 | shareFile(filePath: string, fileType: string, callback: Function) { | 287 | shareFile(filePath: string, fileType: string, callback: Function) { |
| 261 | - const uri = fileUri.getUriFromPath(filePath) | 288 | + try { |
| 262 | - Log.debug(`shareFile uri: ${uri}, filePath: ${filePath}`) | 289 | + const uri = fileUri.getUriFromPath(filePath) |
| 263 | - this.start(uri, fileType, callback) | 290 | + Log.debug(`shareFile uri: ${uri}, filePath: ${filePath}`) |
| 291 | + this.start(uri, fileType, callback) | ||
| 292 | + } catch (e) { | ||
| 293 | + Log.debug(`shareFile err:${JSON.stringify(e)}`) | ||
| 294 | + callback(`shareFile err:${JSON.stringify(e)}`) | ||
| 295 | + } | ||
| 264 | } | 296 | } |
| 265 | start(uri: string, fileType: string, callback: Function) { | 297 | start(uri: string, fileType: string, callback: Function) { |
| 266 | const mimeType = getMimeType(fileType) | 298 | const mimeType = getMimeType(fileType) |
| @@ -1,6 +1,6 @@ | |||
| 1 | { | 1 | { |
| 2 | "name": "@react-native-ohos/react-native-doc-viewer", | 2 | "name": "@react-native-ohos/react-native-doc-viewer", |
| 3 | - "version": "2.8.2-rc.1", | 3 | + "version": "2.8.3-beta.1", |
| 4 | "description": "React Native Native Module Bridge Quicklock Document Viewer for IOS + Android supports pdf, png, jpg, xls, ppt, doc, docx, pptx, xlx + Video Player mp4 supported ", | 4 | "description": "React Native Native Module Bridge Quicklock Document Viewer for IOS + Android supports pdf, png, jpg, xls, ppt, doc, docx, pptx, xlx + Video Player mp4 supported ", |
| 5 | "main": "lib/commonjs/index", | 5 | "main": "lib/commonjs/index", |
| 6 | "scripts": { | 6 | "scripts": { |
【AI-Review】【一般】【基础代码问题】【稳定性问题】watchDownloadTask fail 回调 removeFile 未 await，cache=true 重试可能分享残缺文件
● 问题：
watchDownloadTask的 'fail' 回调（turborModules.ts:280-284）在 line 282 调用this.removeFile(filePath)未使用await，紧接着 line 283 调用callback(download fail, code:${err})。removeFile（line 177）是async方法，内部需要await fs.access+await fs.unlink才能真正删除残缺文件。由于未await，callback在残缺文件被删除前就通知 JS 侧。若 JS 侧以cache=true重试同一 URL，useCache（line 157）在 line 164 执行await fs.access(filePath)时，残缺文件可能仍然存在，isExists为 true，useCache在 line 167 调用this.shareFile(filePath, fileType, callback)分享的是上一次下载失败的残缺文件而非重新下载的完整文件。本 PR 已在download（line 255）为removeFile补了await修复同类竞态，但watchDownloadTask的 'fail' 回调中相同的removeFile调用未同步修复。● 影响：一般。下载大文件失败后，JS 侧以
cache=true快速重试同一 URL 的时序场景下，removeFile的fs.unlink尚未完成，useCache的fs.access读到残缺文件并直接分享，用户会打开到上一次下载的残缺文件而非重新下载的完整文件。下载小文件时removeFile完成快，影响较小。● 建议：将 'fail' 回调改为
async并await this.removeFile(filePath)，确保残缺文件在通知 JS 侧前已删除： downloadTask.on('fail', async (err: number) => { Log.debug(download fail:${fileName}, code:${err}); await this.removeFile(filePath); callback(download fail, code:${err}); });