[riot-notifications] [RIOT-OS/RIOT] gnrc_tcp: allow unknown options (#12298)

Martine Lenders notifications at github.com
Tue Sep 24 19:38:43 CEST 2019


miri64 requested changes on this pull request.



> @@ -51,7 +51,9 @@ int _option_parse(gnrc_tcp_tcb_t *tcb, tcp_hdr_t *hdr)
                 continue;
 
             case TCP_OPTION_KIND_MSS:
-                if (opt_left < TCP_OPTION_LENGTH_MIN || option->length > opt_left || option->length != TCP_OPTION_LENGTH_MSS) {
+                if (opt_left < TCP_OPTION_LENGTH_MIN || option->length > opt_left ||
+                    option->length != TCP_OPTION_LENGTH_MSS) {
+                    

Unrelated change, but okay.

> @@ -61,14 +63,15 @@ int _option_parse(gnrc_tcp_tcb_t *tcb, tcp_hdr_t *hdr)
                 break;
 
             default:
-                DEBUG("gnrc_tcp_option.c : _option_parse() : Unknown option found.\
-                      KIND=%"PRIu8", LENGTH=%"PRIu8"\n", option->kind, option->length);
-                return -1;
+                if (TCP_OPTION_LENGTH_MIN <= opt_left) {

This confused me, because it is the other way around below. I'd prefer
```suggestion
                if (opt_left >= TCP_OPTION_LENGTH_MIN) {
```

>          }
 
         if (opt_left < TCP_OPTION_LENGTH_MIN || option->length > opt_left) {
-            DEBUG("gnrc_tcp_option.c : _option_parse() : invalid option length\n");
-            return 0;
+            DEBUG("gnrc_tcp_option.c : _option_parse() : Invalid option. Drop Packet.\n");
+            return -1;

Is it okay to merge the two different return values here?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/12298#pullrequestreview-292591098
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190924/61bf01f5/attachment.htm>


More information about the notifications mailing list