Skip to content
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.

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

Removed dependency on 'org.apache.commons.codec' #7460

Merged

Conversation

cweitkamp
Copy link
Contributor

Related to openhab/openhab-core#1436

Signed-off-by: Christoph Weitkamp [email protected]

@cweitkamp cweitkamp added the infrastructure Build system and Karaf related issues and PRs label Apr 24, 2020
@cweitkamp cweitkamp requested review from kaikreuzer and a team April 24, 2020 10:19
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling it will break the Freebox binding.

// Covert array of Hex bytes to a String
return new String(hexBytes, StandardCharsets.UTF_8);
// Convert raw bytes to a String
return HexUtils.bytesToHex(rawHmac);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling this will not be the same result.
The new call will add a space between each hexa value while the previous code did not.

Copy link
Contributor Author

@cweitkamp cweitkamp Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will work - check it out. I wrote a unit test for each change.

    @Test
    public void testFreebox() {
        byte[] rawSubject = "test".getBytes(StandardCharsets.UTF_8);

        byte[] hexBytes = new Hex().encode(rawSubject);
        String subject = new String(hexBytes, StandardCharsets.UTF_8);

        assertThat(subject, is(HexUtils.bytesToHex(rawSubject)));
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if you are sure, that is fine for me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lolodomo Then please remove your "changes requested" review :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to suppress it !
I approved it even if I check only the part relative to the Freebox binding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems you made it :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I approved it globally but only reviewed one part.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks - you're the best 😎!

@kaikreuzer kaikreuzer merged commit 332847f into openhab:2.5.x Apr 24, 2020
@cweitkamp cweitkamp deleted the feature-remove-apache-commons-codec branch April 24, 2020 12:03
@cpmeister cpmeister added this to the 2.5.5 milestone Apr 24, 2020
yfre pushed a commit to yfre/openhab-addons that referenced this pull request Apr 27, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
Signed-off-by: Eugen Freiter <[email protected]>
@lolodomo
Copy link
Contributor

@cweitkamp : just after installing version 2.5.5, I discovered the authentication to the Freebox is no more working. I am afraid you have broken the binding with your change in the method hmacSha1 !

@lolodomo
Copy link
Contributor

If I restore the previous code in the method hmacSha1, the binding is working again.

@lolodomo
Copy link
Contributor

lolodomo commented May 19, 2020

Here is the comparison with the old code and your new code:

20:03:08.989 [DEBUG] [reebox.internal.api.FreeboxApiManager] - hmacSha1 new code: *0F7480528064D7F06AA363F4E9646C83B9CFCCD1*
20:03:08.995 [DEBUG] [reebox.internal.api.FreeboxApiManager] - hmacSha1 old code: *0f7480528064d7f06aa363f4e9646c83b9cfccd1*
20:03:09.001 [DEBUG] [reebox.internal.api.FreeboxApiManager] - hmacSha1 old == new: false

The difference is the case. I will apply a lowercase to the result with the new code to see if it works.

@cpmeister
Copy link
Contributor

If you find a fix, it would be a good idea to make a unit test out of it.

lolodomo added a commit to lolodomo/openhab-addons that referenced this pull request May 19, 2020
PR openhab#7460 broke the freebox binding

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo
Copy link
Contributor

@cweitkamp : where is your unit test, I don't find it ?

@cweitkamp
Copy link
Contributor Author

@lolodomo I am sorry to break it. See #7460 (comment) for the unit test. I did not commit it anywhere. Just executed it locally.

IIRC I had to transform the result to lower case in another case too.

@lolodomo
Copy link
Contributor

In case it broke several bindings, it could be necessary to produce a new distribution 2.5.6 very quickly

cpmeister pushed a commit that referenced this pull request May 20, 2020
* [freebox] Hot fix for login to the freebox
PR #7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
* Removed dependency on 'org.apache.commons.codec'
* Replaced usage of 'StringUtils'

Signed-off-by: Christoph Weitkamp <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
* [freebox] Hot fix for login to the freebox
PR openhab#7460 broke the freebox binding
* Unit test added for method hmacSha1

Signed-off-by: Laurent Garnier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Build system and Karaf related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants