Invalidate alias with null character #282
Merged
Conversation
This file contains 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
kulla
commented
Mar 23, 2021
| @@ -216,6 +216,23 @@ describe('uuid', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| test('returns an error when alias contains the null character', async () => { | |||
There was a problem hiding this comment.
Is it the right approach to return an error instead of null? This would mean:
- CF: Entry would not end in the CF cache and the CF worker would use the legacy backend ✔️
- Frontend: Would not display uuid and report this to sentry
- API: Would report the error to sentry
There was a problem hiding this comment.
Sounds good. This error should only occur if both the cache value and the therefore newly fetched value are both invalid.
kulla
commented
Mar 23, 2021
| @@ -40,10 +40,16 @@ export const InstanceDecoder: t.Type<Instance> = t.union([ | |||
| t.literal(Instance.Ta), | |||
| ]) | |||
|
|
|||
| const StringWithoutNullCharacter = t.refinement( | |||
There was a problem hiding this comment.
Here I use the deprecated t.refinement() instead of t.brand() due to gcanti/io-ts#576 since we would otherwise have a ts compiler error at https://github.com/serlo/api.serlo.org/blob/master/src/schema/uuid/video/resolvers.ts#L33
See serlo/database-layer#80 for an example
kulla
force-pushed
the
decoder-refinements
branch
from
e539736
to
9f37c60
Compare
inyono
approved these changes
Mar 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR includes a refinement for the alias string which does not allow the null character. This would remove all invalid cache entries with null characters in the alias and thus would force a refresh for them (see serlo/database-layer#80 and https://serlo.slack.com/archives/C0BMSC431/p1616161728010900 for examples). When the database layer returns such an invalid uuid an error would given by the API.