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

Migration to a maintained version of the Apache HTTP Client #168

Closed
mgmechanics opened this issue Jun 18, 2018 · 14 comments
Closed

Migration to a maintained version of the Apache HTTP Client #168

mgmechanics opened this issue Jun 18, 2018 · 14 comments
Milestone

Comments

@mgmechanics
Copy link
Contributor

Zest contains Apache HTTP Client 3.1 which is unmaintained since long time ago. Is there serious interest to update it to Apache HTTP Client 4 or better?

The company I am working for uses Zest internally. It is doing great but our source scan raises security errors of highest level because of the outdated HTTP client. Thus I already updated the HTTP Client to version 4.5.5. If there serious interest I can ask if I can merge my changes into the public code base.

@thc202
Copy link
Member

thc202 commented Jun 18, 2018

I'm +1 to the change, but worth noting that it will break binary compatibility (if we completely drop Commons HttpClient).

@psiinon
Copy link
Member

psiinon commented Jun 19, 2018

+1 as well.
Do we really need binary compatibility?
Zest script compatibility is the important thing to maintain :)

@thc202
Copy link
Member

thc202 commented Jun 19, 2018

Well, this change will break (ZAP) Zest add-on, but not a problem we can update it easily :)

@mgmechanics
Copy link
Contributor Author

mgmechanics commented Jun 20, 2018

Zest script compatibility is the important thing to maintain :)

I can run the same Zest scripts than before the change. org.mozilla.zest.core.v1.ZestRequest works with class Cookie from the Apache HTTP Client 4.5.5.

This is possible because JSON from the Class Cookie from HTTP Client 3.1 can be de-serialised in to objects of the class Cookie in the Apache HTTP Client 4.5.5.

Everything else in the package org.mozilla.zest.core.v1 was not changed.

@mgmechanics
Copy link
Contributor Author

The changes took place in the class org.mozilla.zest.impl.ZestBasicRunner. There are some bigger changes because before migration to HTTP Client 4 I isolated all methods from this class using the HTTP Client 3 in a new class. ZestBasicRunner holds an interface of this class which gets an instance of the new class at runtime. This makes further migrations (Apache HTTP Client 5 is in the beta version) easier.

@mgmechanics
Copy link
Contributor Author

I ask my boss if I can get a budget for this.

@thc202
Copy link
Member

thc202 commented Jun 20, 2018

Which version/commit are you basing your work on? Asking because #118 refactored ZestBasicRunner to allow that.

@mgmechanics
Copy link
Contributor Author

I did the migration on the master branch. I have checked #118. That looks good. You have already done an important part of the work.

A) ZestCookie: Good idea. Makes Zest independent from particular HTTP clients.

B) org.mozilla.zest.impl.CommonsHttpClient implements ZestHttpClient: Almost the same as my approach. I think I can begin. Have to ask for procedure/budget first.

@thc202
Copy link
Member

thc202 commented Jun 21, 2018

Note that the work should be done in the develop branch (master is being used only for tagging releases).

Cool, let us know how that goes.

@mgmechanics
Copy link
Contributor Author

Of course, in the develop branch.

@mgmechanics
Copy link
Contributor Author

I did the work and made the pull request #171

@mgmechanics
Copy link
Contributor Author

Tests were done successfully by running examples/BodgeIt_Register_XSS.zst against a dummy service so I could check the requests made by the zest runner.

@mgmechanics
Copy link
Contributor Author

Do you plan to include the new HTTP client for the next release?

@thc202 thc202 added this to the 0.14.0 milestone Aug 24, 2018
@thc202
Copy link
Member

thc202 commented Aug 24, 2018

Sure, good to get this change in (added to milestone).

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

No branches or pull requests

3 participants