You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by shukitchan <gi...@git.apache.org> on 2016/02/28 08:59:22 UTC
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/503
TS-4240: fix coredump issues with esi plugin
@jpeach @zwoop pls take a look and review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shukitchan/trafficserver esi_utils
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/503.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #503
----
commit 0acb3b60be6ae61c405b9dbf48897f4bb4a58383
Author: Kit Chan <ki...@apache.org>
Date: 2016-02-28T07:57:55Z
TS-4240: fix coredump issues with esi plugin
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-210513483
I'm -1 on this unless we know it actually fixes the problem. Looking at the [TS-4240](https://issues.apache.org/jira/browse/TS-4240), my guess is that ``data[i]`` is negative, so casting to a wider unsigned type gives you a very large index. The right fix would be to cast to ``uint8_t``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-210846959
@jpeach patch is updated. please take a look.
I will test more tonight and if it is good, i will squash the commit before merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/503
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-216435937
Ship it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-210377071
@jpeach i thought that it was safe, too. Till i see the core dump reported in the jira. i am just being extra safe in this PR. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-216400714
I can now reproduce the problem and the fix is working.
@jpeach pls take a look as well
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-216405109
@shukitchan does it make sense to add something to the esi unit tests for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-199875223
@shukitchan Can you address Jame's comments, and then commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...
Posted by shukitchan <gi...@git.apache.org>.
Github user shukitchan commented on the pull request:
https://github.com/apache/trafficserver/pull/503#issuecomment-216434585
test added. @jpeach pls take a look.
again i will squash the commits before merging. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---