-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(PdfReader): add SetPdfStreamAsync/SetPdfBase64DataAsync instance method #817
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -132,7 +132,7 @@ public partial class PdfReader | |||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||
| /// <remarks>优先使用 <see cref="Url"/> 未提供 <see cref="Url"/> 时会尝试调用此回调获得流进行渲染</remarks> | ||||||||||||||||||||||||
| [Parameter] | ||||||||||||||||||||||||
| public Func<Task<Stream>>? OnGetStreamAsync { get; set; } | ||||||||||||||||||||||||
| public Func<Task<Stream?>>? OnGetStreamAsync { get; set; } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| [Inject, NotNull] | ||||||||||||||||||||||||
| private IStringLocalizer<PdfReader>? Localizer { get; set; } | ||||||||||||||||||||||||
|
|
@@ -156,6 +156,8 @@ public partial class PdfReader | |||||||||||||||||||||||
| private bool _enableThumbnails = true; | ||||||||||||||||||||||||
| private bool _showToolbar = true; | ||||||||||||||||||||||||
| private PdfReaderFitMode _fitMode; | ||||||||||||||||||||||||
| private string _lastStreamHash = string.Empty; | ||||||||||||||||||||||||
| private long _lastStreamLength = 0; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||
| /// <inheritdoc/> | ||||||||||||||||||||||||
|
|
@@ -188,14 +190,16 @@ protected override async Task OnAfterRenderAsync(bool firstRender) | |||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _url = Url; | ||||||||||||||||||||||||
| _currentPage = CurrentPage; | ||||||||||||||||||||||||
| _enableThumbnails = EnableThumbnails; | ||||||||||||||||||||||||
| _currentRotation = CurrentRotation; | ||||||||||||||||||||||||
| _showToolbar = ShowToolbar; | ||||||||||||||||||||||||
| _enableThumbnails = EnableThumbnails; | ||||||||||||||||||||||||
| _fitMode = FitMode; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (_url != Url) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _url = Url; | ||||||||||||||||||||||||
| _lastStreamHash = string.Empty; | ||||||||||||||||||||||||
| await InvokeVoidAsync("setUrl", Id, _url); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if (_currentPage != CurrentPage) | ||||||||||||||||||||||||
|
|
@@ -229,8 +233,124 @@ protected override async Task OnAfterRenderAsync(bool firstRender) | |||||||||||||||||||||||
| _fitMode = FitMode; | ||||||||||||||||||||||||
| await SetFitMode(_fitMode); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if (string.IsNullOrEmpty(Url)) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| Stream? stream = null; | ||||||||||||||||||||||||
| if (OnGetStreamAsync != null) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| stream = await OnGetStreamAsync(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| await SetPdfStream(stream); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private async Task SetPdfStream(Stream? stream) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if (stream == null || stream == Stream.Null) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _lastStreamHash = string.Empty; | ||||||||||||||||||||||||
| _lastStreamLength = 0; | ||||||||||||||||||||||||
| await InvokeVoidAsync("setData", Id, null); | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| byte[] pdfBytes = await GetBytes(stream); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| var currentLength = pdfBytes.Length; | ||||||||||||||||||||||||
| if (_lastStreamLength != currentLength) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| _lastStreamLength = currentLength; | ||||||||||||||||||||||||
| await InvokeVoidAsync("setData", Id, pdfBytes); | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #if NET6_0 | ||||||||||||||||||||||||
| var currentHash = ComputerHash(pdfBytes); | ||||||||||||||||||||||||
| #else | ||||||||||||||||||||||||
| var currentHash = await ComputerHash(stream); | ||||||||||||||||||||||||
|
Comment on lines
+269
to
+271
|
||||||||||||||||||||||||
| var currentHash = ComputerHash(pdfBytes); | |
| #else | |
| var currentHash = await ComputerHash(stream); | |
| var currentHash = ComputeHash(pdfBytes); | |
| #else | |
| var currentHash = await ComputeHash(stream); |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method name typo: ComputerHash should be ComputeHash.
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hash computation logic is incorrect for non-NET6_0 targets. On line 271, you're trying to compute the hash of stream after it has already been fully read and converted to bytes on line 258. The stream position will be at the end, making the hash computation inaccurate or computing the hash of an already-consumed stream.
Since you already have pdfBytes available, you should compute the hash from the byte array instead. You can create a synchronous overload of ComputerHash that accepts byte[] for non-NET6_0 targets, or convert the conditional compilation to use the same byte-based approach for both targets.
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XML documentation for the stream parameter is missing. Public API methods should have complete documentation including parameter descriptions. Add a <param name="stream"> tag to describe what the stream parameter represents (e.g., "The PDF file stream to load").
| /// <param name="stream"></param> | |
| /// <param name="stream">The PDF file stream to load.</param> |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method name typo: ComputerHash should be ComputeHash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Using the original stream for hashing after copying introduces similar seekability issues and inconsistency with the byte-based path.
In SetPdfStreamAsync, you copy stream into memoryStream but, for non-NET6, you still compute _lastStreamHash from the original stream. As with SetPdfStream, this fails for non-seekable streams (you’ll hash only the remaining tail, often empty) and the hash won’t match the rendered pdfBytes. To keep behavior correct and consistent, compute the hash from pdfBytes for both NET6 and non-NET6 instead of using the original stream.
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method name typo: ComputerHash should be ComputeHash.
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hash computation logic is incorrect for non-NET6_0 targets. On line 320, you're trying to compute the hash of the original stream parameter after it has already been fully read and copied to memoryStream on line 314. At this point, the stream position is at the end, and even if it's seekable, it will compute the hash of an empty or already-consumed stream.
You should either:
- Compute the hash from
pdfBytesinstead ofstream(like you do for NET6_0) - Pass
memoryStreamto the hash function and ensure it's seeked back to the beginning
The simplest fix is to use ComputerHash(pdfBytes) for the non-NET6_0 case as well, since you already have the byte array.
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XML documentation for the base64Data parameter is missing. Public API methods should have complete documentation including parameter descriptions. Add a <param name="base64Data"> tag to describe what the parameter represents (e.g., "The Base64-encoded PDF data").
| /// <param name="base64Data"></param> | |
| /// <param name="base64Data">The Base64-encoded PDF data.</param> |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SetPdfBase64DataAsync method doesn't update the _lastStreamHash and _lastStreamLength fields like SetPdfStreamAsync does. This inconsistency means that if the component state tracking logic depends on these fields, calling this method could lead to unexpected behavior or redundant updates. Consider whether these fields should be updated here for consistency, or document why they don't need to be.
| var pdfBytes = Convert.FromBase64String(base64Data); | |
| var pdfBytes = Convert.FromBase64String(base64Data); | |
| _lastStreamLength = pdfBytes.Length; | |
| #if NET6_0 | |
| _lastStreamHash = ComputerHash(pdfBytes); | |
| #else | |
| using (var memoryStream = new MemoryStream(pdfBytes)) | |
| { | |
| _lastStreamHash = await ComputerHash(memoryStream); | |
| } | |
| #endif |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name has a typo: ComputerHash should be ComputeHash. "Computer" is a noun (the machine), while "Compute" is the verb (to calculate). This typo appears in all three occurrences of the method (lines 337, 343, and their usages).
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name has a typo: ComputerHash should be ComputeHash. This is the async overload version of the same incorrectly named method.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,7 +36,6 @@ export async function setUrl(id, url) { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| const { options } = pdf; | ||||||||||||||||||||||
| options.url = url; | ||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): Switching from a blob-based data source to a URL does not revoke any existing object URL, which can leak memory.
|
||||||||||||||||||||||
| options.data = null; | ||||||||||||||||||||||
| await loadPdf(pdf); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -48,12 +47,33 @@ export async function setData(id, data) { | |||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const { options } = pdf; | ||||||||||||||||||||||
| options.url = null; | ||||||||||||||||||||||
| options.data = data; | ||||||||||||||||||||||
| const { options, objectUrl } = pdf; | ||||||||||||||||||||||
| if (objectUrl) { | ||||||||||||||||||||||
| URL.revokeObjectURL(objectUrl); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| options.url = createObjectURLFromByte(data); | ||||||||||||||||||||||
| options.data = null; | ||||||||||||||||||||||
| pdf.objectUrl = options.url; | ||||||||||||||||||||||
|
Comment on lines
+50
to
+57
|
||||||||||||||||||||||
| await loadPdf(pdf); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const createObjectURLFromBase64 = base64Data => { | ||||||||||||||||||||||
| const binaryString = atob(base64Data); | ||||||||||||||||||||||
| const bytes = new Uint8Array(binaryString.length); | ||||||||||||||||||||||
| for (let i = 0; i < binaryString.length; i++) { | ||||||||||||||||||||||
| bytes[i] = binaryString.charCodeAt(i); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const blob = new Blob([bytes], { type: 'application/pdf' }); | ||||||||||||||||||||||
| return URL.createObjectURL(blob); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+61
to
+70
|
||||||||||||||||||||||
| const createObjectURLFromBase64 = base64Data => { | |
| const binaryString = atob(base64Data); | |
| const bytes = new Uint8Array(binaryString.length); | |
| for (let i = 0; i < binaryString.length; i++) { | |
| bytes[i] = binaryString.charCodeAt(i); | |
| } | |
| const blob = new Blob([bytes], { type: 'application/pdf' }); | |
| return URL.createObjectURL(blob); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Hashing the original stream in the non-NET6 path is fragile for non-seekable streams and unnecessarily re-reads the stream.
In
SetPdfStream, the non-NET6 branch hashes the originalstreamafterGetByteshas fully read it. For non-seekable streams this meansComputerHash(stream)runs at end-of-stream and produces an incorrect hash, and for seekable streams it forces a second pass. Since you already havepdfBytes, consider hashingpdfBytesin both NET6 and non-NET6 builds to avoid dependence on stream seekability and double-reading.