[jira] [Commented] (KAFKA-4930) Connect Rest API allows creating connectors with an empty name

["Randall Hauch (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-4930?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246460#comment-16246460 ] 

Randall Hauch commented on KAFKA-4930:
--------------------------------------

[~sliebau] created KIP-212 and then started the [KIP discussion on the AK dev mailing list|https://mail-archives.apache.org/mod_mbox/kafka-dev/201710.mbox/%3CCAMTc-3GXD3jYfknq3LHV726OFMRkbVyGjwnNAsOJL=UxaP0MNg@mail.gmail.com%3E]. After a few questions and time to research, Sönke reports the following:

{quote}
I've spent some time looking at this and testing various characters and it would appear that Randall's suspicion was spot on. I think we can support a fairly large set of characters with very minor changes.

I was put of by the exceptions that were thrown when creating connectors with certain characters and suspected a larger underlying problem when in fact the only issue is, that the URL in the rest request used to retrieve the response for the create connector request needs to be percent encoded (see [ConnectorsResource.java|https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java#L102]).

I've fixed this and done some local testing which worked out quite nicely, apart from two special cases, I've not been able to find characters that created issues, even space and slash work.
The mentioned special cases are:
* {{\}}  - if the name contains a backslash that is not the beginning of a valid escape sequence the request fails before we ever get it in ConnectorsResource, so a backslash would need to be escaped: \\
* {{"}}  - Quotation marks need to be escaped as well to keep the json body of the request legal: \"

In both cases the escape character will be part of the connector name and need to be specified in the url to retrieve the connector as well, even though we could URL encode it in a legal way without escaping here. So they work, not sure if I'd recommend using those characters, but no real reason to prohibit people from using them that I can see either.

What I'd do going forward is:
- withdraw the KIP, as I don't see a real need for one, since this is not changing anything, just fixing things.
- add a section to the documentation around legal characters, specify the ones I tested explicitly (url encoded %20 - %7F) and mention that most other characters should work as well but no guarantees are given
- update the pull request for KAFKA-4930 to allow all characters but still prohibit creating a connector with an empty name. I'd propose to keep the validator though as it'll give us a central location to do any checking that might turn out to be necessary later on.
- add some integration tests to check connectors with special characters in their names work
- fix the url encoding line in ConnectorsResource

Does that sound fair to everybody?
{quote}

The important takeaway: Connect names should be URL-encoded in the REST request and will then be properly handled by Connect. The only characters that need to be handled differently (before URL-encoding the connector name) are the double quote character {{"}} and backslash character {{\\}} that must be *_escaped_* with a preceding backslash character within the connector name _before_ it is URL-encoded.

> Connect Rest API allows creating connectors with an empty name
> --------------------------------------------------------------
>
>                 Key: KAFKA-4930
>                 URL: https://issues.apache.org/jira/browse/KAFKA-4930
>             Project: Kafka
>          Issue Type: Bug
>          Components: KafkaConnect
>    Affects Versions: 0.10.2.0
>            Reporter: Sönke Liebau
>            Assignee: Sönke Liebau
>            Priority: Minor
>
> The Connect Rest API allows to deploy connectors with an empty name field, which then cannot be removed through the api.
> Sending the following request:
> {code}
> {
>     "name": "",
>     "config": {
>         "connector.class": "org.apache.kafka.connect.tools.MockSourceConnector",
>         "tasks.max": "1",
>         "topics": "test-topic"
>         
>     }
> }
> {code}
> Results in a connector being deployed which can be seen in the list of connectors:
> {code}
> [
> 	"",
> 	"testconnector"
> ]{code}
> But cannot be removed via a DELETE call, as the api thinks we are trying to delete the /connectors endpoint and declines the request.
> I don't think there is a valid case for the connector name to be empty so perhaps we should add a check for this. I am happy to work on this.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)