Sonke,

Have you had a chance to update the KIP and kick off a VOTE thread? We need
to do this ASAP if we want this to make the KIP deadline for 1.1, which is
Jan 23!

On Tue, Jan 16, 2018 at 10:33 PM, Ewen Cheslack-Postava <e...@confluent.io>
wrote:

> Sonke,
>
> I'm fine filtering some control characters. The trimming also seems like it
> might be *somewhat* moot because the way connector names work in standalone
> mode is limited by ConfigDef, which already does trimming of settings. Not
> a great reason to be restrictive, but we'd partly just be codifying what's
> there.
>
> I just generally have a distaste for being restrictive without a clear
> reason. In this case I don't think it has any significant impact.
>
> KIP freeze is nearing and this seems like a simple improvement and a PR is
> already available (modulo any changes re: control characters). I'll start
> reviewing the PR, do you want to make any last updates about control
> characters in the KIP and kick off a VOTE thread?
>
> -Ewen
>
> On Fri, Jan 12, 2018 at 1:43 PM, Colin McCabe <cmcc...@apache.org> wrote:
>
> > On Fri, Jan 12, 2018, at 08:03, Sönke Liebau wrote:
> > > Hi everybody,
> > >
> > > from reading the discussion I understand that we have two things still
> > > open for discussen.
> > >
> > >  Ewen is still a bit on the fence about whether or not we trim
> > > whitespace characters but seems to favor not doing it due to there not
> > > being a real issue with them. I think it mostly boils down to a
> > > question of general preference. I am happy to change the code to allow
> > > leading and trailing whitespace characters again. If someone has a use
> > > case for these, so be it - I don't see a technical reason against
> > > them. Personally I think it is more likely that someone accidentally
> > > gets a whitespace character in his connector name somehow and
> > > subsequently has a confusing time figuring it out, but it shouldn't be
> > > that tough to spot and is correct behavior, so no issue with it.
> > > TL/DR: I'm happy either way :)
> > >
> > > Colin brought up control characters and that we should disallow these
> > > in connector names. The specific one that is mentioned in his link is
> > > Ascii 27 - ESC - \e so one possibility would be to explicitly
> > > blacklist this. The rest of the control characters (Ascii 0 through 31
> > > and 127) should be less critical I think, but since there is no way of
> > > knowing all software that might look at these strings and interpret
> > > them there is no real way of being certain. I think there is a case to
> > > be made for disallowing all control characters (and their respective
> > > escape sequences where applicable) in connector names - perhaps with
> > > the possible exclusion of /n /r /t .
> >
> > +1
> >
> > Colin
> >
> > >
> > > Thoughts?
> > >
> > > Kind regards,
> > > Sönke
> > >
> > >
> > >
> > > On Wed, Jan 10, 2018 at 7:23 AM, Ewen Cheslack-Postava
> > > <e...@confluent.io> wrote:
> > > > great point, I'm always for exclusions where they make sense. i just
> > prefer
> > > > to include by default w/ exclusions when necessary to listing
> explicit
> > > > inclusions and being restrictive. (and security updates immediately
> as
> > > > needed).
> > > >
> > > > If you have a set of characters you think we should exclude, I think
> it
> > > > would be good to add them here or in a subsequent KIP!
> > > >
> > > > -Ewen
> > > >
> > > > On Tue, Jan 9, 2018 at 1:30 PM, Colin McCabe <cmcc...@apache.org>
> > wrote:
> > > >
> > > >> On Sat, Jan 6, 2018, at 16:00, Ewen Cheslack-Postava wrote:
> > > >> > re: whitespace characters, I'm fine with the restriction since I
> > don't
> > > >> see
> > > >> > it becoming an issue in practice. I just don't see any reason to
> > restrict
> > > >> > it so it seems like we're going out of our way and doing extra
> work
> > to be
> > > >> > restrictive, but without clear motivation.
> > > >>
> > > >> There are very good reasons not to support control characters in
> file
> > > >> names, topic names, log files, etc.
> > > >>
> > > >> See http://seclists.org/fulldisclosure/2003/Feb/att-
> > 341/Termulation.txt
> > > >>
> > > >> There are a bunch of CVEs about this, too.  Because of the (in my
> > opinion,
> > > >> mistaken) decision to allow control characters in UNIX filenames,
> even
> > > >> echoing a file name to your terminal is a security vulnerability.
> > > >>
> > > >> best,
> > > >> Colin
> > > >>
> > > >>
> > > >> >
> > > >> > In general my default approach (without context of a specific
> > system)
> > > >> would
> > > >> > be to accept anything that we can encode in UTF-8 and only apply
> > > >> > restrictions where it becomes necessary (e.g. we need to define a
> > > >> delimiter
> > > >> > for some reason). The constraints of URLs introduce some
> complexity
> > (you
> > > >> > need escaping), but probably generally still allow this. If I can
> > use an
> > > >> > emoji when naming things, then I'm probably happy :) Whitespace
> > > >> characters
> > > >> > definitely have some other issues (e.g. you can have non-visible
> > > >> whitespace
> > > >> > which obscures which connector you're actually working with), but
> > despite
> > > >> > the JIRA linked, I wasn't really convinced they need special
> > handling. It
> > > >> > seems like a really weird issue to encounter in the first place.
> > > >> >
> > > >> > -Ewen
> > > >> >
> > > >> > On Fri, Jan 5, 2018 at 8:10 AM, Randall Hauch <rha...@gmail.com>
> > wrote:
> > > >> >
> > > >> > > Sönke, I'm happy with the current proposal.
> > > >> > >
> > > >> > > Ewen, the proposal allows any characters in the name as long as
> > they
> > > >> are
> > > >> > > properly escaped/encoded. That seems to adhere to the robustness
> > > >> principle.
> > > >> > > The only exception is that the proposal trims leading and
> trailing
> > > >> > > whitespace characters in an attempt to reduce user errors. Can
> you
> > > >> please
> > > >> > > clarify that you're okay with this behavior? I agree that
> > technically
> > > >> we
> > > >> > > can (and currently do) support whitespace-only names, but users
> > have
> > > >> > > reported this as problematic, and it also would be confusing for
> > most
> > > >> user
> > > >> > > interfaces.
> > > >> > >
> > > >> > > Best regards,
> > > >> > >
> > > >> > > Randall
> > > >> > >
> > > >> > > On Thu, Jan 4, 2018 at 10:31 PM, Ewen Cheslack-Postava <
> > > >> e...@confluent.io>
> > > >> > > wrote:
> > > >> > >
> > > >> > > > Very late to the game here, but a few thoughts:
> > > >> > > >
> > > >> > > > 1. Regarding whether KIP is necessary, I don't mind doing it
> for
> > > >> > > > documentation sake, but I would classify any mishandling of
> > connector
> > > >> > > names
> > > >> > > > here as a bug. Which doesn't require a KIP to fix.
> > > >> > > >
> > > >> > > > 2. For support of characters, Kafka has some history of just
> > being
> > > >> > > > restrictive (e.g., see topic name restrictions), but I
> > personally
> > > >> > > disagree
> > > >> > > > with this approach. I think it is better to be liberal in what
> > we
> > > >> accept
> > > >> > > > and just document limitations. I think our default should be
> to
> > > >> accept
> > > >> > > any
> > > >> > > > user input and document why we can't handle certain inputs and
> > how
> > > >> the
> > > >> > > user
> > > >> > > > should adapt if we can't. In general I try to work under the
> > > >> robustness
> > > >> > > > principle: *Be conservative in what you do, be liberal in what
> > you
> > > >> accept
> > > >> > > > from others*
> > > >> > > >
> > > >> > > > 3. Related to 2, there were some cases like whitespace-only
> > connector
> > > >> > > > names. This seems extremely weird and not critical, so I'm
> fine
> > not
> > > >> > > > supporting it officially, but technically I don't see any
> > reason it
> > > >> > > > shouldn't be supported with any appropriate escaping (i.e.
> what
> > > >> would it
> > > >> > > > break for us?).
> > > >> > > >
> > > >> > > > But in general, I think just being more explicit about
> > expectations
> > > >> is
> > > >> > > > great and it'd be great to set baseline expectations.
> > > >> > > >
> > > >> > > > -Ewen
> > > >> > > >
> > > >> > > >
> > > >> > > >
> > > >> > > > On Mon, Nov 20, 2017 at 12:33 AM, Sönke Liebau <
> > > >> > > > soenke.lie...@opencore.com.invalid> wrote:
> > > >> > > >
> > > >> > > > > @Randall: are you happy with the KIP as it stands so I can
> > call
> > > >> for a
> > > >> > > > vote,
> > > >> > > > > or are there any outstanding items still to discuss?
> > > >> > > > >
> > > >> > > > > Same question to anybody else who'd like to participate of
> > course
> > > >> :)
> > > >> > > > >
> > > >> > > > > On Thu, Nov 16, 2017 at 5:35 PM, Sönke Liebau <
> > > >> > > > soenke.lie...@opencore.com>
> > > >> > > > > wrote:
> > > >> > > > >
> > > >> > > > > > Sounds good. I've added a few sentences to this effect to
> > the
> > > >> KIP.
> > > >> > > > > >
> > > >> > > > > > On Thu, Nov 16, 2017 at 5:02 PM, Randall Hauch <
> > rha...@gmail.com
> > > >> >
> > > >> > > > wrote:
> > > >> > > > > >
> > > >> > > > > >> Nice job updating the KIP. The PR (
> > > >> > > > > >> https://github.com/apache/kafka/pull/2755/files) for the
> > > >> proposed
> > > >> > > > > >> implementation does prevent names from being empty, and
> it
> > trims
> > > >> > > > > >> whitespace
> > > >> > > > > >> from the name only when creating a new connector.
> However,
> > the
> > > >> KIP's
> > > >> > > > > >> "Proposed Change" section should probably be very clear
> > about
> > > >> this,
> > > >> > > > and
> > > >> > > > > >> the
> > > >> > > > > >> migration section should address how a connector that was
> > > >> created
> > > >> > > with
> > > >> > > > > >> leading and/or trailing whitespace characters will still
> be
> > > >> able to
> > > >> > > be
> > > >> > > > > >> updated and deleted. I think that decreases the
> likelihood
> > of
> > > >> this
> > > >> > > > > change
> > > >> > > > > >> negatively impacting existing users. Basically, going
> > forward,
> > > >> the
> > > >> > > > names
> > > >> > > > > >> of
> > > >> > > > > >> new connectors will be trimmed.
> > > >> > > > > >>
> > > >> > > > > >> WDYT?
> > > >> > > > > >>
> > > >> > > > > >> On Thu, Nov 16, 2017 at 9:32 AM, Sönke Liebau <
> > > >> > > > > >> soenke.lie...@opencore.com.invalid> wrote:
> > > >> > > > > >>
> > > >> > > > > >> > I've added some more detail to the KIP [1] around
> current
> > > >> > > scenarios
> > > >> > > > > that
> > > >> > > > > >> > might break in the future. I actually came up with a
> > second
> > > >> > > > limitation
> > > >> > > > > >> that
> > > >> > > > > >> > we'd impose on users and also documented this.
> > > >> > > > > >> >
> > > >> > > > > >> > Let me know what you think.
> > > >> > > > > >> >
> > > >> > > > > >> > Kind regards,
> > > >> > > > > >> > Sönke
> > > >> > > > > >> >
> > > >> > > > > >> > [1]
> > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >> > > > > >> > 212%3A+Enforce+set+of+legal+
> > characters+for+connector+names
> > > >> > > > > >> >
> > > >> > > > > >> >
> > > >> > > > > >> > On Thu, Nov 16, 2017 at 2:59 PM, Sönke Liebau <
> > > >> > > > > >> soenke.lie...@opencore.com>
> > > >> > > > > >> > wrote:
> > > >> > > > > >> >
> > > >> > > > > >> > > Hi Randall,
> > > >> > > > > >> > >
> > > >> > > > > >> > > I had mentioned this edge case in the KIP, but will
> > add some
> > > >> > > > further
> > > >> > > > > >> > > detail to further clarify all changing scenarios post
> > pull
> > > >> > > > request.
> > > >> > > > > >> > >
> > > >> > > > > >> > > Kind regards,
> > > >> > > > > >> > > Sönke
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > > On Thu, Nov 16, 2017 at 2:06 PM, Randall Hauch <
> > > >> > > rha...@gmail.com>
> > > >> > > > > >> wrote:
> > > >> > > > > >> > >
> > > >> > > > > >> > >> No, we need to keep the KIP since we want to
> > > >> change/correct the
> > > >> > > > > >> existing
> > > >> > > > > >> > >> behavior. But we do need to clarify in the KIP these
> > edge
> > > >> cases
> > > >> > > > > that
> > > >> > > > > >> > will
> > > >> > > > > >> > >> change.
> > > >> > > > > >> > >>
> > > >> > > > > >> > >> Thanks for the continued work on this, Sönke.
> > > >> > > > > >> > >>
> > > >> > > > > >> > >> Regards,
> > > >> > > > > >> > >>
> > > >> > > > > >> > >> Randall
> > > >> > > > > >> > >>
> > > >> > > > > >> > >> > On Nov 16, 2017, at 1:56 AM, Sönke Liebau <
> > > >> > > > > >> soenke.lie...@opencore.com
> > > >> > > > > >> > >> .INVALID> wrote:
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > Hi Randall,
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > zero length definitely works, that's what sent me
> > down
> > > >> this
> > > >> > > > hole
> > > >> > > > > in
> > > >> > > > > >> > the
> > > >> > > > > >> > >> > first place. I had a customer accidentally create
> a
> > > >> connector
> > > >> > > > > >> without
> > > >> > > > > >> > a
> > > >> > > > > >> > >> > name in his environment and then be unable to
> > delete it.
> > > >> No
> > > >> > > > > >> connector
> > > >> > > > > >> > >> name
> > > >> > > > > >> > >> > doesn't work, as this throws a null pointer
> > exception
> > > >> due to
> > > >> > > > > >> > KAFKA-4938
> > > >> > > > > >> > >> ,
> > > >> > > > > >> > >> > but once that is fixed would create a connector
> > named
> > > >> "null"
> > > >> > > I
> > > >> > > > > >> think.
> > > >> > > > > >> > >> Have
> > > >> > > > > >> > >> > not retested this, but seen it in the past.
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > Also, it is possible to create connectors with
> > trailing
> > > >> and
> > > >> > > > > leading
> > > >> > > > > >> > >> > whitespaces, this errors out on the create request
> > (which
> > > >> > > will
> > > >> > > > be
> > > >> > > > > >> > fixed
> > > >> > > > > >> > >> > when KAFKA-4827 is merged), but correctly creates
> > the
> > > >> > > connector
> > > >> > > > > and
> > > >> > > > > >> > you
> > > >> > > > > >> > >> can
> > > >> > > > > >> > >> > access it if you percent-escape the curl call.
> This
> > for
> > > >> me is
> > > >> > > > the
> > > >> > > > > >> main
> > > >> > > > > >> > >> > reason why a KIP might be needed, as we are
> changing
> > > >> public
> > > >> > > > > facing
> > > >> > > > > >> > >> behavior
> > > >> > > > > >> > >> > here. I agree with you, that this will probably
> not
> > > >> affect
> > > >> > > > anyone
> > > >> > > > > >> or
> > > >> > > > > >> > >> hardly
> > > >> > > > > >> > >> > anyone, but in principle it is a change that
> should
> > need
> > > >> a
> > > >> > > KIP
> > > >> > > > I
> > > >> > > > > >> > think.
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > I've retested and documented this for Confluent
> > 3.3.0:
> > > >> > > > > >> > >> > https://gist.github.com/soenkeliebau/
> > > >> 9363745cff23560fcc234d9
> > > >> > > > > >> b64ac14c4
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > I am of course happy to withdraw the KIP if you
> > think it
> > > >> is
> > > >> > > > > >> > unnecessary,
> > > >> > > > > >> > >> > I've also updated the pull request for KAFKA-4930
> to
> > > >> reflect
> > > >> > > > the
> > > >> > > > > >> > changes
> > > >> > > > > >> > >> > stated in the KIP and tested the code with Arjuns
> > pull
> > > >> > > request
> > > >> > > > > for
> > > >> > > > > >> > >> > KAFKA-4827 to ensure they don't interfere with
> each
> > > >> other.
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > Let me know what you think.
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > Kind regards,
> > > >> > > > > >> > >> > Sönke
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > ᐧ
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> >> On Tue, Nov 14, 2017 at 7:03 PM, Randall Hauch <
> > > >> > > > > rha...@gmail.com>
> > > >> > > > > >> > >> wrote:
> > > >> > > > > >> > >> >>
> > > >> > > > > >> > >> >> Thanks for updating the KIP to reflect the
> current
> > > >> process.
> > > >> > > > > >> However,
> > > >> > > > > >> > I
> > > >> > > > > >> > >> >> still question whether it is necessary to have a
> > KIP -
> > > >> it
> > > >> > > > > depends
> > > >> > > > > >> on
> > > >> > > > > >> > >> >> whether it was possible with prior versions to
> have
> > > >> > > connectors
> > > >> > > > > >> with
> > > >> > > > > >> > >> >> zero-length or blank names. Have you tried both
> of
> > these
> > > >> > > > cases?
> > > >> > > > > >> > >> >>
> > > >> > > > > >> > >> >> On Fri, Nov 10, 2017 at 3:52 AM, Sönke Liebau <
> > > >> > > > > >> > >> >> soenke.lie...@opencore.com.invalid> wrote:
> > > >> > > > > >> > >> >>
> > > >> > > > > >> > >> >>> Hi Randall,
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>> I have set aside some time to work on this next
> > week.
> > > >> The
> > > >> > > fix
> > > >> > > > > >> itself
> > > >> > > > > >> > >> is
> > > >> > > > > >> > >> >>> quite simple, but I've yet to write tests to
> > properly
> > > >> catch
> > > >> > > > > this,
> > > >> > > > > >> > >> which
> > > >> > > > > >> > >> >>> turns out to be a bit more complex, as it needs
> a
> > > >> running
> > > >> > > > > >> restserver
> > > >> > > > > >> > >> >> which
> > > >> > > > > >> > >> >>> is mocked in the tests I've looked at so far.
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>> Should I withdraw the KIP or update it to
> reflect
> > the
> > > >> > > > > >> documentation
> > > >> > > > > >> > >> >> changes
> > > >> > > > > >> > >> >>> and enforced rules around trimming and zero
> length
> > > >> > > connector
> > > >> > > > > >> names?
> > > >> > > > > >> > >> This
> > > >> > > > > >> > >> >> is
> > > >> > > > > >> > >> >>> a change to existing behavior, even if it is
> quite
> > > >> small
> > > >> > > and
> > > >> > > > > >> > probably
> > > >> > > > > >> > >> >> won't
> > > >> > > > > >> > >> >>> even be noticed by many people..
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>> best regards,
> > > >> > > > > >> > >> >>> Sönke
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>>> On Thu, Nov 9, 2017 at 9:10 PM, Randall Hauch <
> > > >> > > > > rha...@gmail.com
> > > >> > > > > >> >
> > > >> > > > > >> > >> wrote:
> > > >> > > > > >> > >> >>>>
> > > >> > > > > >> > >> >>>> Any progress on updating the PR and withdrawing
> > > >> KIP-212?
> > > >> > > > > >> > >> >>>>
> > > >> > > > > >> > >> >>>> On Fri, Oct 27, 2017 at 5:19 PM, Randall Hauch
> <
> > > >> > > > > >> rha...@gmail.com>
> > > >> > > > > >> > >> >> wrote:
> > > >> > > > > >> > >> >>>>
> > > >> > > > > >> > >> >>>>> Yes, connector names should not be blank or
> > contain
> > > >> just
> > > >> > > > > >> > whitespace.
> > > >> > > > > >> > >> >> In
> > > >> > > > > >> > >> >>>>> fact, I might recommend that we trim
> whitespace
> > at
> > > >> the
> > > >> > > > front
> > > >> > > > > >> and
> > > >> > > > > >> > >> rear
> > > >> > > > > >> > >> >>> of
> > > >> > > > > >> > >> >>>>> new connector names and then disallowing any
> > > >> zero-length
> > > >> > > > > name.
> > > >> > > > > >> > >> >> Existing
> > > >> > > > > >> > >> >>>>> connectors would remain valid, and this would
> > not
> > > >> break
> > > >> > > > > >> backward
> > > >> > > > > >> > >> >>>>> compatibility. That might require a small kip
> > simply
> > > >> to
> > > >> > > > > update
> > > >> > > > > >> the
> > > >> > > > > >> > >> >>>>> documentation and specify what names are
> valid.
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>> WDYT?
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>> Randall
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>> On Fri, Oct 27, 2017 at 1:08 PM, Colin McCabe
> <
> > > >> > > > > >> cmcc...@apache.org
> > > >> > > > > >> > >
> > > >> > > > > >> > >> >>>> wrote:
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>>>> On Wed, Oct 25, 2017, at 01:07, Sönke Liebau
> > wrote:
> > > >> > > > > >> > >> >>>>>>> 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
> > > >> > > > > >> > >> >>>>>>> [1].
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> 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.
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>> Good research, Sönke.
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> 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.
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>> Are empty names currently allowed?  That's
> > > >> unfortunate.
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>>> - 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?
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>> It sounds good to me, but I will let someone
> > more
> > > >> > > > > >> knowledgeable
> > > >> > > > > >> > >> >> about
> > > >> > > > > >> > >> >>>>>> connect chime in.
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>> best,
> > > >> > > > > >> > >> >>>>>> Colin
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> Kind regards,
> > > >> > > > > >> > >> >>>>>>> Sönke
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> [1]
> > > >> > > > > >> > >> >>>>>>> https://github.com/apache/
> > > >> kafka/blob/trunk/connect/
> > > >> > > > > runtime/
> > > >> > > > > >> > >> >>>>>> src/main/java/org/apache/
> > > >> kafka/connect/runtime/rest/
> > > >> > > > > >> > >> >>>>>> resources/ConnectorsResource.java#L102
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> On Tue, Oct 24, 2017 at 8:40 PM, Colin
> McCabe
> > <
> > > >> > > > > >> > cmcc...@apache.org
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>>>>> wrote:
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> On Tue, Oct 24, 2017, at 11:28, Sönke
> Liebau
> > > >> wrote:
> > > >> > > > > >> > >> >>>>>>>>> Hi,
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> after reading your messages I'll grant
> that
> > I
> > > >> might
> > > >> > > > have
> > > >> > > > > >> > >> >> picked
> > > >> > > > > >> > >> >>> a
> > > >> > > > > >> > >> >>>>>>>>> somewhat
> > > >> > > > > >> > >> >>>>>>>>> draconic option to solve these issues.
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> In general I believe that properly
> encoding
> > the
> > > >> URLs
> > > >> > > > > after
> > > >> > > > > >> > >> >>> having
> > > >> > > > > >> > >> >>>>>> created
> > > >> > > > > >> > >> >>>>>>>>> the connectors should solve a lot of the
> > issues
> > > >> > > > already.
> > > >> > > > > >> For
> > > >> > > > > >> > >> >>> some
> > > >> > > > > >> > >> >>>>>>>>> characters the rest api returns an error
> on
> > > >> creating
> > > >> > > > the
> > > >> > > > > >> > >> >>> connector
> > > >> > > > > >> > >> >>>>>> as
> > > >> > > > > >> > >> >>>>>>>>> well,
> > > >> > > > > >> > >> >>>>>>>>> so for that URL encoding won't help.
> > However the
> > > >> > > > > >> connectors do
> > > >> > > > > >> > >> >>> get
> > > >> > > > > >> > >> >>>>>>>>> created
> > > >> > > > > >> > >> >>>>>>>>> even though an error is returned, I've
> never
> > > >> > > > investigated
> > > >> > > > > >> if
> > > >> > > > > >> > >> >>> they
> > > >> > > > > >> > >> >>>>>> are in
> > > >> > > > > >> > >> >>>>>>>>> a
> > > >> > > > > >> > >> >>>>>>>>> consistent state tbh - I'll give this
> > another
> > > >> look.
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> @colin: Entity encoding would allow us to
> > encode
> > > >> a
> > > >> > > lot
> > > >> > > > of
> > > >> > > > > >> > >> >>>>>> characters,
> > > >> > > > > >> > >> >>>>>>>>> however I am unsure whether we should
> > prefer it
> > > >> over
> > > >> > > > url
> > > >> > > > > >> > >> >>> encoding
> > > >> > > > > >> > >> >>>>>> in this
> > > >> > > > > >> > >> >>>>>>>>> case, as mostly the end user would have to
> > > >> encode the
> > > >> > > > > >> > >> >> characters
> > > >> > > > > >> > >> >>>>>> himself.
> > > >> > > > > >> > >> >>>>>>>>> And due to entity encoding ending every
> > character
> > > >> > > with
> > > >> > > > a
> > > >> > > > > ;
> > > >> > > > > >> > >> >> which
> > > >> > > > > >> > >> >>>>>> causes
> > > >> > > > > >> > >> >>>>>>>>> the
> > > >> > > > > >> > >> >>>>>>>>> embedded jetty server to cut the connector
> > name
> > > >> at
> > > >> > > that
> > > >> > > > > >> > >> >>> character
> > > >> > > > > >> > >> >>>>>> we'd
> > > >> > > > > >> > >> >>>>>>>>> probably need to encode that character in
> > URL
> > > >> > > encoding
> > > >> > > > > >> again
> > > >> > > > > >> > >> >> for
> > > >> > > > > >> > >> >>>>>> that to
> > > >> > > > > >> > >> >>>>>>>>> work out - which might get a bit too
> > complex tbh.
> > > >> > > > > >> > >> >>>>>>>>
> > > >> > > > > >> > >> >>>>>>>> Sorry, I meant to write percent-encoding,
> not
> > > >> entity
> > > >> > > > refs.
> > > >> > > > > >> > >> >>>>>>>> https://en.wikipedia.org/wiki/
> > Percent-encoding
> > > >> > > > > >> > >> >>>>>>>>
> > > >> > > > > >> > >> >>>>>>>> best,
> > > >> > > > > >> > >> >>>>>>>> Colin
> > > >> > > > > >> > >> >>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> I will further investigate which
> characters
> > the
> > > >> url
> > > >> > > > > >> decoding
> > > >> > > > > >> > >> >>> that
> > > >> > > > > >> > >> >>>>>> jetty
> > > >> > > > > >> > >> >>>>>>>>> brings to the table will let us use and if
> > all of
> > > >> > > these
> > > >> > > > > are
> > > >> > > > > >> > >> >>>>>> correctly
> > > >> > > > > >> > >> >>>>>>>>> handled during connector creation and
> > report back
> > > >> > > with
> > > >> > > > a
> > > >> > > > > >> new
> > > >> > > > > >> > >> >>> list
> > > >> > > > > >> > >> >>>> of
> > > >> > > > > >> > >> >>>>>>>>> characters that I think we can support
> > fairly
> > > >> easily.
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> Kind regards,
> > > >> > > > > >> > >> >>>>>>>>> Sönke
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> On Tue, Oct 24, 2017 at 6:42 PM, Colin
> > McCabe <
> > > >> > > > > >> > >> >>> cmcc...@apache.org
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>>>>> wrote:
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>> It should be possible to use entity
> > references
> > > >> to
> > > >> > > > encode
> > > >> > > > > >> > >> >> these
> > > >> > > > > >> > >> >>>>>>>>>> characters in URLs.  See
> > > >> > > > https://dev.w3.org/html5/html-
> > > >> > > > > >> > >> >>>>>> author/charref
> > > >> > > > > >> > >> >>>>>>>>>> Maybe I'm misunderstanding the problem,
> > but can
> > > >> we
> > > >> > > > > simply
> > > >> > > > > >> > >> >>> encode
> > > >> > > > > >> > >> >>>>>> the
> > > >> > > > > >> > >> >>>>>>>>>> URLs, rather than restricting the names?
> > > >> > > > > >> > >> >>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>> best,
> > > >> > > > > >> > >> >>>>>>>>>> Colin
> > > >> > > > > >> > >> >>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>> On Mon, Oct 23, 2017, at 14:12, Randall
> > Hauch
> > > >> > > wrote:
> > > >> > > > > >> > >> >>>>>>>>>>> Here's the link to KIP-212:
> > > >> > > > > >> > >> >>>>>>>>>>> https://cwiki.apache.org/
> > > >> confluence/pages/viewpage
> > > >> > > .
> > > >> > > > > >> > >> >>>>>>>>>> action?pageId=74684586
> > > >> > > > > >> > >> >>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>> I do think it's worthwhile to define the
> > rules
> > > >> for
> > > >> > > > > >> > >> >> connector
> > > >> > > > > >> > >> >>>>>> names.
> > > >> > > > > >> > >> >>>>>>>>>>> However, I think it would be better to
> > > >> describe the
> > > >> > > > > >> > >> >> current
> > > >> > > > > >> > >> >>>>>>>> restrictions
> > > >> > > > > >> > >> >>>>>>>>>>> for names outside of them appearing
> within
> > > >> URLs.
> > > >> > > For
> > > >> > > > > >> > >> >>> example,
> > > >> > > > > >> > >> >>>>>> if we
> > > >> > > > > >> > >> >>>>>>>> can
> > > >> > > > > >> > >> >>>>>>>>>>> keep connector names relatively free of
> > > >> constraints
> > > >> > > > but
> > > >> > > > > >> > >> >>>> instead
> > > >> > > > > >> > >> >>>>>>>> define
> > > >> > > > > >> > >> >>>>>>>>>>> how
> > > >> > > > > >> > >> >>>>>>>>>>> names should be encoded when used within
> > URLs
> > > >> > > (e.g.,
> > > >> > > > > URL
> > > >> > > > > >> > >> >>>>>> encoding),
> > > >> > > > > >> > >> >>>>>>>> then
> > > >> > > > > >> > >> >>>>>>>>>>> we
> > > >> > > > > >> > >> >>>>>>>>>>> may not have (m)any backward
> compatibility
> > > >> issues
> > > >> > > > other
> > > >> > > > > >> > >> >> than
> > > >> > > > > >> > >> >>>>>> fixing
> > > >> > > > > >> > >> >>>>>>>> some
> > > >> > > > > >> > >> >>>>>>>>>>> bugs related to proper
> encoding/decoding.
> > > >> > > > > >> > >> >>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>> Thoughts?
> > > >> > > > > >> > >> >>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>> On Mon, Oct 23, 2017 at 3:44 PM, Sönke
> > Liebau <
> > > >> > > > > >> > >> >>>>>>>>>>> soenke.lie...@opencore.com.invalid>
> > wrote:
> > > >> > > > > >> > >> >>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>>> All,
> > > >> > > > > >> > >> >>>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>>> I've created a KIP to discuss enforcing
> > of
> > > >> rules
> > > >> > > on
> > > >> > > > > what
> > > >> > > > > >> > >> >>>>>>>> characters are
> > > >> > > > > >> > >> >>>>>>>>>>>> allowed in connector names.
> > > >> > > > > >> > >> >>>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>>> Since this may break api calls that are
> > > >> currently
> > > >> > > > > >> > >> >> working
> > > >> > > > > >> > >> >>> I
> > > >> > > > > >> > >> >>>>>>>> figured a
> > > >> > > > > >> > >> >>>>>>>>>> KIP
> > > >> > > > > >> > >> >>>>>>>>>>>> is the better way to go than to just
> > create a
> > > >> > > jira.
> > > >> > > > > >> > >> >>>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>>> I'd love to hear your input on this!
> > > >> > > > > >> > >> >>>>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>>
> > > >> > > > > >> > >> >>>>>>>>> --
> > > >> > > > > >> > >> >>>>>>>>> Sönke Liebau
> > > >> > > > > >> > >> >>>>>>>>> Partner
> > > >> > > > > >> > >> >>>>>>>>> Tel. +49 179 7940878
> > > >> > > > > >> > >> >>>>>>>>> OpenCore GmbH & Co. KG -
> Thomas-Mann-Straße
> > 8 -
> > > >> 22880
> > > >> > > > > >> Wedel -
> > > >> > > > > >> > >> >>>>>> Germany
> > > >> > > > > >> > >> >>>>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>>
> > > >> > > > > >> > >> >>>>>>> --
> > > >> > > > > >> > >> >>>>>>> Sönke Liebau
> > > >> > > > > >> > >> >>>>>>> Partner
> > > >> > > > > >> > >> >>>>>>> Tel. +49 179 7940878
> > > >> > > > > >> > >> >>>>>>> OpenCore GmbH & Co. KG - Thomas-Mann-Straße
> 8
> > -
> > > >> 22880
> > > >> > > > > Wedel -
> > > >> > > > > >> > >> >>> Germany
> > > >> > > > > >> > >> >>>>>>
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>>
> > > >> > > > > >> > >> >>>>
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>> --
> > > >> > > > > >> > >> >>> Sönke Liebau
> > > >> > > > > >> > >> >>> Partner
> > > >> > > > > >> > >> >>> Tel. +49 179 7940878 <+49%20179%207940878>
> > > >> > > > > >> > >> >>> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 -
> > 22880
> > > >> > > Wedel -
> > > >> > > > > >> > Germany
> > > >> > > > > >> > >> >>>
> > > >> > > > > >> > >> >>
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> >
> > > >> > > > > >> > >> > --
> > > >> > > > > >> > >> > Sönke Liebau
> > > >> > > > > >> > >> > Partner
> > > >> > > > > >> > >> > Tel. +49 179 7940878 <+49%20179%207940878>
> > > >> > > > > >> > >> > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 -
> > 22880
> > > >> Wedel -
> > > >> > > > > >> Germany
> > > >> > > > > >> > >>
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > >
> > > >> > > > > >> > > --
> > > >> > > > > >> > > Sönke Liebau
> > > >> > > > > >> > > Partner
> > > >> > > > > >> > > Tel. +49 179 7940878 <+49%20179%207940878>
> > > >> > > > > >> > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> > Wedel
> > > >> -
> > > >> > > > > Germany
> > > >> > > > > >> > >
> > > >> > > > > >> >
> > > >> > > > > >> >
> > > >> > > > > >> >
> > > >> > > > > >> > --
> > > >> > > > > >> > Sönke Liebau
> > > >> > > > > >> > Partner
> > > >> > > > > >> > Tel. +49 179 7940878
> > > >> > > > > >> > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> > Wedel -
> > > >> > > > Germany
> > > >> > > > > >> >
> > > >> > > > > >>
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > --
> > > >> > > > > > Sönke Liebau
> > > >> > > > > > Partner
> > > >> > > > > > Tel. +49 179 7940878 <+49%20179%207940878>
> > > >> > > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
> Wedel
> > -
> > > >> Germany
> > > >> > > > > >
> > > >> > > > >
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > --
> > > >> > > > > Sönke Liebau
> > > >> > > > > Partner
> > > >> > > > > Tel. +49 179 7940878
> > > >> > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel
> -
> > > >> Germany
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> > >
> > >
> > >
> > > --
> > > Sönke Liebau
> > > Partner
> > > Tel. +49 179 7940878
> > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany
> >
>

Reply via email to