Relax validation on user-specified topic aliasing #334
Conversation
| local_publish_view.topic_alias = &outbound_topic_alias; | ||
| } else { |
There was a problem hiding this comment.
Trivial: We could probably do without this log. I may be missing something but I don't think it's providing any useful information and the absence of the topic alias log above should be enough to infer that the publish isn't using topic aliasing. Also unsure whether we need to NULL
I think local_publish_view.topic_alias = NULL may be unnecessary. if outbound_topic_alias == 0, it's already NULL or an error was raised by the resolver. Could be missing something deeper tho.
There was a problem hiding this comment.
null assignment is necessary; up until now we're using what is specified by the user which could be an invalid mapping.
There was a problem hiding this comment.
As an addendum, we're always in a better position if the logging tells us exactly what happened on its own, rather than having to read the log and the code and infer what happened based on what didn't show up in the logs.
| @@ -1743,30 +1743,6 @@ static int s_aws_mqtt5_packet_publish_view_validate_vs_connection_settings( | |||
| return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION); | |||
| } | |||
|
|
|||
| if (publish_view->topic_alias != NULL) { | |||
| const struct aws_mqtt5_client_options_storage *client_options = client->config; | |||
| if (client_options->topic_aliasing_options.outbound_topic_alias_behavior != AWS_MQTT5_COTABT_USER) { | |||
There was a problem hiding this comment.
Trivial: Not erroring here seems fine but should we log that we're ignoring?
There was a problem hiding this comment.
I feel like encoder logging gives a definitive answer to what we do alias-wise.
Being strict about topic alias validation seems counter productive. It's an optional micro-optimization and the client configuration may not be known to a user who may have to work with a client configured by a third party.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.