Skip to content

Conversation

wujimin
Copy link
Contributor

@wujimin wujimin commented Sep 19, 2017

unify http request to standard HttpServletRequest, not private interface

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 84.992% when pulling 10e7028 on wujimin:unify-http-request into e6c04f0 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 85.23% when pulling 6523574 on wujimin:unify-http-request into e6c04f0 on ServiceComb:master.

@WillemJiang
Copy link
Member

@wujimin Could you take a look the code coverage report? It looks like there are some lines are not covered.

@wujimin
Copy link
Contributor Author

wujimin commented Sep 19, 2017

yes, i added so many test case in this PR, but coverage decreased......
i'm handing this problem from this afternoon.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 85.223% when pulling 142c718 on wujimin:unify-http-request into 528b018 on ServiceComb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 85.245% when pulling df647ac on wujimin:unify-http-request into 528b018 on ServiceComb:master.

@coveralls
Copy link

coveralls commented Sep 19, 2017

Coverage Status

Coverage decreased (-0.3%) to 85.327% when pulling 9840278 on wujimin:unify-http-request into 528b018 on ServiceComb:master.

@wujimin
Copy link
Contributor Author

wujimin commented Sep 19, 2017

simpler some implements, and delete many useless class in new implements, maybe this cause the coverage decreased
from coverage detail, we can see all coverages for this PR file increased

if (serverSignature != null && !signature.equals(serverSignature)) {
return Response.create(Status.UNAUTHORIZED, "signature failed");
if (serverSignature != null) {
LOGGER.info("check response signature, client: {}, server: {}.", signature, serverSignature);
Copy link
Contributor

Choose a reason for hiding this comment

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

sinature不要记录到日志里面。 属于敏感信息内容

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just demo, not product

try {
String signature = SignatureUtils.genSignature(request.getRequestURI(), invocation.getContext(), request);
String clientSignature = invocation.getContext("signature");
LOGGER.info("check request signature, client: {}, server: {}.", clientSignature, signature);
Copy link
Contributor

Choose a reason for hiding this comment

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

sinature不要记录到日志里面。 属于敏感信息内容

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just demo, not product

@WillemJiang WillemJiang merged commit 71c8ce2 into apache:master Sep 20, 2017
@wujimin wujimin deleted the unify-http-request branch September 22, 2017 06:30
@wujimin wujimin changed the title Unify http request JAV-349 Unify http request Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants