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

Revert "move invalid carfile tests to separate PR" #1162

Merged
merged 4 commits into from
Sep 11, 2023

Conversation

gvelez17
Copy link
Contributor

@gvelez17 gvelez17 commented Sep 11, 2023

This reverts commit d82ee7b.

add test for invalid carfile having 'left' signal

test('throws error on invalid car file', () => {
const validation = parser.parse(CAR_FILE_INVALID as ExpReq)
console.log("Here is invalid validation: " + JSON.stringify(validation))
expect(() => parser.parse(CAR_FILE_INVALID as ExpReq)).toThrow(/^Can not decode CAR file/);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, this doesn't throw an error, on the junk carfile. instead it happily extracts this stuffs

Here is invalid validation: {"_tag":"Left","left":[{"trail":[{"key":"","type":{"name":"RequestAnchorParamsV2"},"actual":{"0":67,"1":114,"2":101,"3":97,"4":116,"5":101,"6":100,"7":66,"8":121,"9":67,"10":104,"11":97,"12":116,"13":71,"14":80,"15":84,"16":52,"17":89,"18":111,"19":117,"20":99,"21":97,"22":110,"23":85,"24":115,"25":101,"26":84,"27":104,"28":105,"29":115,"30":83,"31":116,"32":114,"33":105,"34":110,"35":103}}]}]}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ukstv i moved it here so i could merge the other one, since this is on some old existing code


test('throws error if cannot decode car file', () => {
const validation = parser.parse(CAR_FILE_NOT_BASE64URL as ExpReq)
console.log("a string of dashes, turns to " + JSON.stringify(validation))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so validation of '---------' results in

{"_tag":"Left","left":[{"trail":[{"key":"","type":{"name":"RequestAnchorParamsV2"},"actual":"----------------------------------------"}]}]}

I don't think then we should even check for https://github.com/ceramicnetwork/ceramic-anchor-service/blob/develop/src/ancillary/anchor-request-params-parser.ts#L99 if we never return any error no matter what?

Copy link
Contributor

@ukstv ukstv Sep 11, 2023

Choose a reason for hiding this comment

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

We return an error. We do not throw it, as it breaks composability. If it returns Left value, then CAR file contains something unexpected.

There is a dozen reasons for an error there inside try-catch block apart just CAR file that is not spec-conformant. It could be an IPLD block of a weird structure, even if it is a pretty valid IPLD object. To maintain composability of codecs, and make it all faster, we wrap a successful result or an error in an Either monad. Here you see Left which is an Either instance that indicates/contains an error.

@gvelez17 gvelez17 marked this pull request as ready for review September 11, 2023 22:21
@gvelez17 gvelez17 merged commit a066900 into feat/extract-cacao Sep 11, 2023
@gvelez17 gvelez17 deleted the feat/test-validator branch September 11, 2023 22:22
gvelez17 added a commit that referenced this pull request Sep 12, 2023
* extract the cacao cid url from signature, or we may use root.tip

Note this is the url, not the actual cacao.  will retrieve async from ipfs

* decode the protected header, add tests

* index not object

* typos - still some issues

* the positive test works now, but invalid carfiles pass?

* move invalid carfile tests to separate PR

* Revert "move invalid carfile tests to separate PR" (#1162)

* Revert "move invalid carfile tests to separate PR"

This reverts commit d82ee7b.

* invalid car file produces isLeft; cannot decode throws error

* ok an invalid carfile has isLeft, but what about some random string?

* remove duplicate coverage test
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