Add punycode encode decode functions #34439

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.

Sign up for GitHub

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

Jump to bottom

Closed

AndreyTokmakov wants to merge 8 commits into ClickHouse:master from AndreyTokmakov:add_punycode_encode_decode_functions

AndreyTokmakov commented Feb 9, 2022 •

edited by alesapin

Loading

Changelog category:

New Feature

Changelog entry:
Implement functions for punycode encoding/decoding: punycodeEncode and punycodeDecode.

Implemented as a wrappers around the pure C version of the implementation:
RFC 3492

SELECT punycodeEncode('правда')

Parameters
- Input string (string to be encoded to punycode)
Return:
- encoded string

SELECT punycodeDecode('xn--mxacd')

Parameters
- Input string (string to be decoded from punycode)
Return:
- decoded string

robot-clickhouse added doc-alert pr-feature labels

CLAassistant commented Feb 9, 2022 •

edited

Loading

All committers have signed the CLA.

AndreyTokmakov mentioned this pull request

Punycode encode/decode. #9437

Closed

KochetovNicolai added the can be tested label

Member

alesapin commented Feb 9, 2022

@Mergifyio update

Contributor

mergify bot commented Feb 9, 2022

update

✅ Branch has been successfully updated

AndreyTokmakov changed the title ~~WIP: Add punycode encode decode functions~~ Add punycode encode decode functions

AndreyTokmakov changed the title ~~Add punycode encode decode functions~~ WIP: Add punycode encode decode functions

novikd self-assigned this

alexey-milovidov unassigned novikd

vitlibar self-assigned this

OrlovDiga commented Jul 25, 2022

Hi everybody!
Is there any information when it will be released?

AndreyTokmakov and others added 7 commits

July 31, 2022 14:20


          Add punycode encode decode functions

72678cf


          Minor code style improvements

656aea6


          Add test .reference file and support decode directly into the output …

4e91432

…buffer


          Add new line at the end of the test files

51e1756


          Fix missing upper to lower convert bug and make std::codecvt a variable

a4e7553


          Support bunch record processing and fix bug with leading slash symbol

5a2bf9f


          Move third-party punycode implementation to contrib.

6130d33

vitlibar force-pushed the add_punycode_encode_decode_functions branch from cea602c to 6130d33 Compare

July 31, 2022 12:25

robot-ch-test-poll added the submodule changed label

vitlibar reviewed

View reviewed changes

contrib/punycode/punycode.h

+              /*
+              punycode-sample.c 2.0.0 (2004-Mar-21-Sun)
+              http://www.nicemice.net/idn/
+              Adam M. Costello

Member

vitlibar Jul 31, 2022 •

edited

Loading

I moved Adam's code to the contrib folder and added a license there (because it's a third-party code and it's better to separate it from our code).

vitlibar reviewed

View reviewed changes

src/Functions/FunctionsPunycode.cpp

+                  }
+                  constexpr bool isASCII(const uint32_t u32) noexcept
+                  {

Member

vitlibar Jul 31, 2022

Too complex, just

bool isASCII(const char32_t c) { return c <= 127; }

is enough

vitlibar reviewed

View reviewed changes

src/Functions/FunctionsPunycode.cpp

+                  {
+                      return (input.length() >= (start + 4)) && (input[start] == 'x' ||input[start] == 'X') &&
+                          (input[start + 1] == 'n' || input[start + 1] == 'N') &&
+                          (input[start + 2] == '-' || input[start + 4] == '-');

Member

vitlibar Jul 31, 2022

That doesn't seem completely correct. "xn--" is the prefix of IDNA encoding and not punycode. I think it's better to have 4 functions instead of 2:

punycodeEncode('ввв') -> 'b1aaa'
punycodeDecode()
idnaEncode('ввв.яндекс.рф') -> 'xn--b1aaa.xn--d1acpjx3f.xn--p1ai'
idnaDecode()

vitlibar reviewed

View reviewed changes

src/Functions/FunctionsPunycode.cpp

+                      if (std::string::npos == start)
+                          return {0, input.size() };
+                      std::string::size_type last = input.find('/', start + 3);
+                      return {start + 3, std::string::npos == last ? input.size() : last};

Member

vitlibar Jul 31, 2022 •

edited

Loading

This function is related to URL processing and not punycode or idna encoding. I believe punycode/idna mustn't try to extract a domain and suppose instead that the domain is already extracted. Anyway we already have the function domain() in ClickHouse which can be easily combined with our function:

SELECT idnaDecode(domain(...))

vitlibar reviewed

View reviewed changes

src/Functions/FunctionsPunycode.cpp

+                  }
+                  size_t punycodeDecode(const std::string_view input,
+                                        UInt8 * decoded_string)

Member

vitlibar Jul 31, 2022

It's better and more safe to pass ColumnString::Chars & res_data to this function instead of UInt8 * decoded_string because this way you can just push back new characters to res_data without thinking about how much memory you need to reserve before.

vitlibar reviewed

View reviewed changes

src/Functions/FunctionsPunycode.cpp

+                      std::for_each(s32.begin(), s32.end(), [](auto & ch) { ch = Poco::Unicode::toLower(ch);});
+                      size_t decoded_length = punycodeEncodeInternal(s32, encoded_string + start);
+                      if (0 == decoded_length)
+                          throw std::runtime_error("Failed to encode");

Member

vitlibar Jul 31, 2022

If something is wrong with arguments of functions usually we throw Exception(ErrorCodes::BAD_ARGUMENTS). Exception is our exception class which provides much more information than std::runtime_error and normally can caught and handled.

alexey-milovidov removed the doc-alert label

alexey-milovidov unassigned vitlibar

pufit self-assigned this

alexey-milovidov unassigned pufit

alexey-milovidov changed the title ~~WIP: Add punycode encode decode functions~~ Add punycode encode decode functions


          Merge branch 'master' into add_punycode_encode_decode_functions

6e1911a

Contributor

robot-clickhouse-ci-2 commented Sep 28, 2023 •

edited by robot-ch-test-poll1

Loading

This is an automated comment for commit 6e1911a with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Successful checks

Check name	Description	Status
Style Check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success

Check name	Description	Status
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	⏳ pending
Docs Check	Builds and tests the documentation	❌ failure

romaincointepas commented Dec 14, 2023

We are highly interested in being able to use those 2 functions (on ClickHouse Cloud we can't use executable UDFs, and this can't be done with SQL UDFs).

rschu1ze mentioned this pull request

Implement punycode encoding/decoding #57969

Merged

Member

rschu1ze commented Dec 17, 2023

I took the freedom and quickly re-implemented this in #57969

rschu1ze closed this

rschu1ze mentioned this pull request

Implement IDNA and punycode encoding/decoding (2nd attempt) #58454

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested pr-feature submodule changed