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

RFC7959 - Block1 Option in Error Response 4.08 (Request Entity Incomplete) #1937

Closed
boaks opened this issue Mar 11, 2022 · 8 comments
Closed
Labels
Announcement for Feedback Announcing a future change in order to get feedback, if that changes should be applied.

Comments

@boaks
Copy link
Contributor

boaks commented Mar 11, 2022

Up to version 3.3.1, including up to version 2.7.0, Californium adds a block1 option to a Error Response 4.08 on blockwise transfers. Though I didn't find an explicit guidance about that in RFC7959, I opened an issue in CorrClar and the current common interpretation is not to use the block1 option for these Error Response.

Therefore I plan to start with

  • adapting that for Californium 3.4.0,
  • and after a period of experience (1-3 months, or so), and if we don't receive an objection, also backport that to a 2.7.1 (considering it as bugfix and not as a behavior break).

Any thoughts on it?

@boaks boaks added the Announcement for Feedback Announcing a future change in order to get feedback, if that changes should be applied. label Mar 11, 2022
boaks pushed a commit to bosch-io/californium that referenced this issue Mar 11, 2022
@sbernard31
Copy link
Contributor

Though I didn't find an explicit guidance about that in RFC7959, I opened an issue in core-wg/corrclar#21 and the current common interpretation is not to use the block1 option for these Error Response.

So I guess it make sense to change the current behavior.

Therefore I plan to start with

  • adapting that for Californium 3.4.0,
  • and after a period of experience (1-3 months, or so), and if we don't receive an objection, also backport that to a 2.7.1 > (considering it as bugfix and not as a behavior break).

I think it make sense to consider it as a bug fix.

Just to let you know :
We only use 2.x in production and we are not able to test this kind of changes with all possible implementation (and version of implementation) we have on the field. So we will provide feed only once we deploy 2.7.1 in production.
It's hard to me to imagine this could be an issue to change this but I was wrong a lot about this kind of bet by the past. We have a lot of different device implementation and sometime they have some very surprising behavior.

So maybe and if possible, it could pay off to let a (not impacting) way to go back to previous behavior with some kind of HACK just in case ? (Maybe making sendBlock1ErrorResponse protected ?)

Or we wait to see if we face some issue then we let you know and then we find a way to implement this kind of HACK.

@boaks
Copy link
Contributor Author

boaks commented Mar 11, 2022

For 2.7.1 we may also go with a configuration.
If removing block1 causes trouble, we can consider to make it also configurable in 3.4+.0.

Anyway, with that, should I also go for a 2.7.1 in short term? If it's the way to get feedback say in Summer or Autumn, I can do that next week and release the 2.7.1 on 17.3.2022. If you prefer to do it in some weeks, that will also be OK for me.

@sbernard31
Copy link
Contributor

For 2.7.1 we may also go with a configuration.

Configuration seems a bit overkill to me to support "bad implementation".

Anyway, with that, should I also go for a 2.7.1 in short term?

For the timing as you prefer. But for sure sooner you release a 2.7.1, more chance to have feedback sooner from us.
(Currently we are using 2.6.6)

@boaks
Copy link
Contributor Author

boaks commented Mar 11, 2022

overkill

Yes. But I'm not sure, if "protected" helps and can be really used. If you're sure, then I would go for that "protected".

It may be also possible, to add such a configuration in 2.8.0 or to remove the fix in 2.7.2.

"bad implementation".

I'm not sure about that. It seems to be more a kind of "interpretation" and someone may unfortunately stick to the block1 option.

@sbernard31
Copy link
Contributor

Yes. But I'm not sure, if "protected" helps and can be really used. If you're sure, then I would go for that "protected".

I just had a quick look and it seems it could be possible to create a custom stack with a custom blockwiseLayer with a custom version of sendBlock1ErrorResponse. but I didn't test it, so not 100% sure too.

I understand that errors, bad interpretations, not clear specifications are real world and so it makes sense to find a way to live with this but I feel (just my option) that configuration is not the right move OR you will add configuration for every possible "bad interpretation" of the specification.

I'm not sure about that. It seems to be more a kind of "interpretation" and someone may unfortunately stick to the block1 option.

I didn't dig this subject so maybe you're right, but if there is nothing in the spec about this and authors clarify the expectation.
I call this "bad implementation". I mean it should not behave like this. (even if it's understandable that some interpret this like that)

It may be also possible, to add such a configuration in 2.8.0 or to remove the fix in 2.7.2.

Yes maybe a better plan to first see if there is problem (or if something was missed about this), then find a way to fix it.

boaks pushed a commit to bosch-io/californium that referenced this issue Mar 11, 2022
@boaks
Copy link
Contributor Author

boaks commented Mar 11, 2022

I schedule a 2.7.1 for next Thursday, 17. March.

boaks pushed a commit that referenced this issue Mar 11, 2022
Fixes: issue #1937

Signed-off-by: Achim Kraus <[email protected]>
boaks pushed a commit that referenced this issue Mar 11, 2022
Fixes: issue #1937

Signed-off-by: Achim Kraus <[email protected]>
@boaks
Copy link
Contributor Author

boaks commented Mar 15, 2022

I remembered, that some time ago BLOCKWISE_STRICT_BLOCK2_OPTION was introduced for something similar at block2.
I have to confess, that I feel more comfortable to add something as BLOCKWISE_STRICT_BLOCK1_OPTION, because that instantly enables user with such clients to use the "newest" Californium version. And adding a warning to the javadoc and the properties doc.

@sbernard31
Copy link
Contributor

I just shared my opinion but feel free to handle it as you prefer.

@boaks boaks closed this as completed Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Announcement for Feedback Announcing a future change in order to get feedback, if that changes should be applied.
Projects
None yet
Development

No branches or pull requests

2 participants