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

9.10.1 The push() method doesn't mention how record data is verified #277

Closed
kenchris opened this issue Aug 5, 2019 · 8 comments
Closed
Labels
Origin Trial Issues that would be good to solve before OT

Comments

@kenchris
Copy link
Contributor

kenchris commented Aug 5, 2019

Like it just states that things should be of type NFCMessageInit, but there is no verification that for instance recordType isn't set to bogus values like "blah-blah". It seems pretty vague, and just trusts that people look at the data mapping table and known how to follow that, but it doesn't even refer to that today

I wonder how we implemented this

@kenchris kenchris added the Origin Trial Issues that would be good to solve before OT label Aug 5, 2019
@riju
Copy link
Collaborator

riju commented Aug 30, 2019

9.10.1 Step 10.14. .. ah too much nesting, basically this step ->

Initiate data transfer to device using output as buffer, using the NFC adapter in communication range with (connected to) device.

Did you mean "overflow" errors? Like developer trying to push more than tag capacity ?
What else should platform verify?

@leonhsl
Copy link
Contributor

leonhsl commented Sep 5, 2019

10.8 of https://w3c.github.io/web-nfc/#the-push-method states:

Let output be the notation for the NDEF message to be created by UA, as the result of passing message to create Web NFC message. If this throws an exception, reject p with that exception and abort these steps.

And 'create Web NFC message' process verifies data of NDEFMessageInit, like an invalid record type as 'blablabla'. See https://w3c.github.io/web-nfc/#creating-web-nfc-message

@zolkis
Copy link
Contributor

zolkis commented Sep 5, 2019

Right now the implementation (and the underlying platform) decide what to do with the input received from a web page.

I suggest we factor out from the push steps the "validate NDEFMessage" steps, eventually relying on "validate NDEFRecord" steps.

@kenchris
Copy link
Contributor Author

kenchris commented Sep 5, 2019

Please do! 👍

zolkis added a commit to zolkis/web-nfc that referenced this issue Sep 5, 2019
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
zolkis added a commit to zolkis/web-nfc that referenced this issue Sep 6, 2019
@zolkis zolkis closed this as completed in f910e4d Sep 10, 2019
@zolkis zolkis reopened this Sep 10, 2019
@zolkis
Copy link
Contributor

zolkis commented Sep 10, 2019

Might be still valid, reopening until confirmed.

@beaufortfrancois
Copy link
Collaborator

@zolkis Can we close it or is this issue still valid?

@zolkis
Copy link
Contributor

zolkis commented Dec 11, 2019

As to the opening comment, I think the algorithms do deal with bogus input (since they react only on valid values and throw otherwise). But it would be good to have a second look at it, maybe @leonhsl could you check the "create" algorithms and close the issue if it's the case?

@leonhsl
Copy link
Contributor

leonhsl commented Dec 12, 2019

https://w3c.github.io/web-nfc/#creating-ndef-message is verifying various user input, in this respect I think we can close this one.

@zolkis zolkis closed this as completed Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Origin Trial Issues that would be good to solve before OT
Projects
None yet
Development

No branches or pull requests

5 participants