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

Add basic functionality for calling and caching the result from the pub.dev api for security advisories #4053

Closed
wants to merge 1 commit into from

Conversation

szakarias
Copy link
Contributor

This is a first step towards showing surfacing security advisories in the client. The functionality is not yet called.

try {
ensureDir(p.dirname(path));

await writeTextFileAsync(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These files are small enough we usually use the sync versions.

path,
jsonEncode(
<String, dynamic>{
...body,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we want to at a timestamp here?

Suggested change
...body,
...body,
'fechedAt': DateTime.now.[convert to string...]

/// /api/packages/$package/advisories
class _PackageAdvisories {
final DateTime? advisoriesUpdated;
final List advisories;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the list needs a type-argument

@@ -537,6 +537,62 @@ class HostedSource extends CachedSource {
return result;
}

Future<_PackageAdvisories> _fetchAdvisories(
_RefAndCache refAndCache,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use the _RefAndCache type, it is only used because records was not a thing back then - just take a Ref and a SystemCache.

body = decoded;

result = _PackageAdvisories(
body['advisories'] as List,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to decode the individual entries?

body = decoded;

result = _PackageAdvisories(
body['advisories'] as List,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot do a type-cast here. We have to validate this is actually a list and throw if not


// Cache the response on disk.
// Don't cache overly big responses.
if (bodyText.length < 500 * 1024) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make this size limit bigger. Perhaps 1000 kb?

? null
: DateTime.parse(body['advisoriesUpdated'] as String),
);
} on Exception catch (error, stackTrace) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be more specific in what we catch here?

@szakarias szakarias marked this pull request as draft November 21, 2023 08:37
@szakarias szakarias closed this Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants