Skip to content

Conversation

karpierz
Copy link
Contributor

@karpierz karpierz commented Mar 22, 2018

Scapy Version: 2.4.0rc5-64
System: Windows7/Windows10
Python Version: 2.7.14/3.4.4/3.6.4

This PR fixes #1199 (regression against scapy<=2.3.2).
(+ some little obvious code improvement in sndrcv())
Please include it in TODO.

See also already closed #1276

This was referenced Mar 22, 2018
@codecov-io
Copy link

codecov-io commented Mar 22, 2018

Codecov Report

Merging #1282 into master will decrease coverage by <.01%.
The diff coverage is 90.9%.

@@            Coverage Diff             @@
##           master    #1282      +/-   ##
==========================================
- Coverage   84.79%   84.79%   -0.01%     
==========================================
  Files         160      160              
  Lines       38322    38340      +18     
==========================================
+ Hits        32495    32510      +15     
- Misses       5827     5830       +3
Impacted Files Coverage Δ
scapy/sendrecv.py 79.52% <90.9%> (+1.26%) ⬆️
scapy/layers/tls/record.py 91.2% <0%> (-0.88%) ⬇️
scapy/layers/tls/handshake_sslv2.py 91.95% <0%> (-0.77%) ⬇️
scapy/asn1/ber.py 82% <0%> (-0.29%) ⬇️

Copy link
Member

@guedou guedou left a comment

Choose a reason for hiding this comment

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

Looks ok to me.

@guedou
Copy link
Member

guedou commented Mar 23, 2018

@gpotter2 @p-l- I approved this PR as it looks OK to me. Could you also review it and approve it if it is OK too ?

@karpierz karpierz changed the title Fix for issue #1199 (regression against scapy<=2.3.2). Please add it to TODO. Fix for issue #1199 (regression against scapy<=2.3.2). Mar 23, 2018
@karpierz karpierz closed this Mar 23, 2018
@karpierz karpierz reopened this Mar 23, 2018
@karpierz karpierz changed the title Fix for issue #1199 (regression against scapy<=2.3.2). Fix for issue #1199 (regression against scapy<=2.3.2). [PR is ready for merge to the master] Mar 23, 2018
@gpotter2
Copy link
Member

Patch is working on single packets, but not lists :/
image

@karpierz
Copy link
Contributor Author

@gpotter2 Fixed

@karpierz karpierz closed this Mar 23, 2018
@karpierz karpierz reopened this Mar 23, 2018
@karpierz karpierz closed this Mar 23, 2018
@karpierz karpierz reopened this Mar 23, 2018
@karpierz karpierz closed this Mar 23, 2018
@karpierz karpierz reopened this Mar 23, 2018
@gpotter2
Copy link
Member

@guedou You should review it again, changes are big

Copy link
Member

Choose a reason for hiding this comment

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

I understand why you removed this try, but this PR itself is already tricky, and it makes it real hard to review + is not needed.
If it’s not bothering you too much, it would be better that you leave the current useless try.

All of those fixes will surely be fixed through the PEPin project, which will come in time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

The logic between ˋsrandsr1` is the same. Because networking tests tend to fail more often, it would be good not to have too many doing the same thing :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@karpierz
Copy link
Contributor Author

PR is ready for merge to the master.

@p-l- p-l- merged commit 94d5521 into secdev:master Mar 24, 2018
@karpierz karpierz deleted the issue_1199_fix branch March 24, 2018 19:30
@karpierz karpierz changed the title Fix for issue #1199 (regression against scapy<=2.3.2). [PR is ready for merge to the master] Fix for issue #1199 (regression against scapy<=2.3.2). Mar 24, 2018
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.

packet.time_sent can't be set because of object copy during list comprehension
5 participants