You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns
Sensitive information exposure: The newly added JobScraper.Tests/Features/Cv/PdfGeneration/CV_content.md contains realistic-looking personal data (name, phone number, email, LinkedIn/GitHub URLs). If this is not clearly synthetic/test-only content, it could be treated as PII in the repository; consider sanitizing or adding an explicit note that the data is fictitious.
Multiple inputs add a Step attribute; please verify it matches the bound property types. In particular, Step="0.1" on integer-backed InputNumber fields can cause parsing/validation issues, and Step on InputText is not meaningful and may produce unexpected markup or warnings.
LinkInline rendering now creates a hyperlink with link.Url ?? "". If Url is null/empty, this can create an invalid/undesired hyperlink target. Consider guarding and falling back to plain text when the URL is missing, and verify the LineBreakInline { IsHard: false } pattern matches the Markdig version used (to avoid missing soft line breaks).
The in-memory SQLite detection builds a SqliteConnectionStringBuilder even when the underlying connection is not SQLite (cast can be null). This can mark non-SQLite connections as “in-memory” due to default/empty values and change behavior compared to the previous check; validate the intended behavior and consider explicitly handling the non-SQLite/null case.
publicstaticasyncTaskPrepareDbAsync(thisIServiceProviderservices){usingvarscope=services.CreateScope();vardbContext=scope.ServiceProvider.GetRequiredService<JobsDbContext>();varsqliteConn=dbContext.Database.GetDbConnection()asSqliteConnection;varcsb=newSqliteConnectionStringBuilder(sqliteConn?.ConnectionString);varisInMemory=csb.Mode==SqliteOpenMode.Memory||csb.DataSourceis"" or ":memory:";if(!isInMemory)EnsureDbDirectoryExists(dbContext);
Avoid constructing SqliteConnectionStringBuilder with a null connection string, which will throw when the underlying connection is not SQLite. Only build the SQLite-specific logic when the connection is actually a SqliteConnection, and skip directory creation for non-SQLite providers.
-var sqliteConn = dbContext.Database.GetDbConnection() as SqliteConnection;-var csb = new SqliteConnectionStringBuilder(sqliteConn?.ConnectionString);-var isInMemory = csb.Mode == SqliteOpenMode.Memory || csb.DataSource is "" or ":memory:";-if (!isInMemory)- EnsureDbDirectoryExists(dbContext);+var dbConnection = dbContext.Database.GetDbConnection();+if (dbConnection is SqliteConnection sqliteConn)+{+ var csb = new SqliteConnectionStringBuilder(sqliteConn.ConnectionString);+ var isInMemory = csb.Mode == SqliteOpenMode.Memory || csb.DataSource is "" or ":memory:";+ if (!isInMemory)+ EnsureDbDirectoryExists(dbContext);+}+
Suggestion importance[1-10]: 8
__
Why: As written, new SqliteConnectionStringBuilder(sqliteConn?.ConnectionString) can throw if GetDbConnection() is not a SqliteConnection (or if the connection string is null), so guarding on dbConnection is SqliteConnection prevents a runtime failure. Skipping EnsureDbDirectoryExists(dbContext) for non-SQLite providers is consistent with this being SQLite-specific setup logic.
Medium
Add fallback for JS import
This path change can break the page at runtime if the JS module file wasn’t renamed to match the new convention. Add a fallback import path so the feature keeps working across environments/build artifacts.
Why: A fallback import can reduce breakage if the JS module path differs between builds, but the blanket catch may hide real module-loading errors and make failures harder to diagnose. This is a defensive compatibility tweak rather than a clear bug fix based on the diff alone.
Low
Security
Validate URLs before linking
Creating hyperlinks with an empty/invalid link.Url can throw or generate broken PDFs, and it can also embed unsafe URI schemes. Validate the URL and fall back to rendering plain text when it’s missing/invalid or uses a disallowed scheme.
case LinkInline link:
- text.Hyperlink(GetInlineText(link), link.Url ?? "")- .FontColor(layoutConfig.UrlColor)- .Underline();+ var label = GetInlineText(link);++ if (Uri.TryCreate(link.Url, UriKind.Absolute, out var uri) &&+ (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps || uri.Scheme == Uri.UriSchemeMailto))+ {+ text.Hyperlink(label, uri.ToString())+ .FontColor(layoutConfig.UrlColor)+ .Underline();+ }+ else+ {+ text.Span(label);+ }+
break;
Suggestion importance[1-10]: 7
__
Why: Validating link.Url before calling text.Hyperlink(...) improves robustness (avoids empty/invalid URLs) and reduces risk of embedding undesirable schemes like javascript: in generated PDFs. The proposed fallback to rendering a plain Span is a reasonable safe behavior when the URL is missing or invalid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.