[Console/Python 3] Remove pyreadline/gnureadline #703

gpotter2 · 2017-06-30T01:49:23Z

This PR is an attempt of removing pyreadline/gnureadline, which have not been updated since (at least) 2015, and are mostly broken under python 3...

Removes pyreadline/gnureadline. Now use IPython to handle the shell.

codecov-io · 2017-06-30T19:08:25Z

Codecov Report

Merging #703 into master will increase coverage by 1.05%.
The diff coverage is 52.57%.

@@            Coverage Diff             @@
##           master     #703      +/-   ##
==========================================
+ Coverage   80.14%   81.19%   +1.05%     
==========================================
  Files         140      141       +1     
  Lines       34207    35139     +932     
==========================================
+ Hits        27416    28532    +1116     
+ Misses       6791     6607     -184

Impacted Files	Coverage Δ
scapy/arch/windows/__init__.py	`78.43% <ø> (ø)`
scapy/config.py	`83.7% <100%> (-0.1%)`	⬇️
scapy/autorun.py	`81.57% <100%> (ø)`	⬆️
scapy/layers/inet.py	`71.45% <100%> (+0.08%)`	⬆️
scapy/utils.py	`78.6% <100%> (+1.17%)`	⬆️
scapy/main.py	`69.66% <46.03%> (+8.89%)`	⬆️
scapy/themes.py	`94.46% <47.82%> (-1.74%)`	⬇️
scapy/layers/tls/record.py	`89.27% <0%> (-2.55%)`	⬇️
scapy/layers/tls/record_sslv2.py	`88.23% <0%> (-1.04%)`	⬇️
scapy/layers/tls/record_tls13.py	`60% <0%> (-0.91%)`	⬇️
... and 24 more

gpotter2 · 2017-07-08T22:59:13Z

@ThomasFaivre Could you have a look at this please ? Thanks!

gpotter2 · 2017-07-08T23:00:56Z

scapy/utils.py

@@ -37,6 +37,9 @@ def get_temp_file(keep=False, autoext=""):
        conf.temp_files.append(f+autoext)
    return f

+class Bunch:
+    __init__ = lambda self, **kw: setattr(self, '__dict__', kw)


This may be used in tests, to create fake objects:

>>> Bunch(a=1).a 1

p-l- · 2017-08-11T09:18:02Z

That looks promising! Can you rebase against master?

gpotter2 · 2017-08-11T10:14:28Z

Rebased.

p-l- · 2017-08-11T13:07:09Z

I have a question for you and @guedou: is it still useful, now, to have this rather than using dedicated shells, like iPython for example. What do you think?

gpotter2 · 2017-08-18T05:15:00Z

I guess IPython could handle it...

We simply need to have an easy backup when it's not available...
(Default shell without the custom read function should do it..)

This would mean that ASCII color codes and auto completion are features only provided through IPython. The current implementation (or the one from this PR) allowed people who don't like IPython to have those two features...

guedou · 2017-08-19T09:05:34Z

I don't have a strong opinion on this topic. As long as the module is easy to install and use on our side, I am fine with any choice. If most of the same features can be achieved with iPython without any modifications in Scapy, that might be the best option.

p-l- · 2017-08-20T00:24:03Z

If people don't like iPython, they have at least bpython as an alternative. I don't think we really need to maintain this interface when great tools exist that do the job (and can run under Linux, *BSD, MacOS & Windows).

@gpotter2 are you one of the people who "don't like iPython"? I'd like to hear what you don't like if that's the case. Thanks!

gpotter2 · 2017-08-22T20:08:57Z

@p-l- I don't like the input system "In[..]" "Out...". I saw there is an option to change this to the "classic legacy" prompt, so i'm ok to use IPython instead.

p-l- · 2017-08-23T06:17:08Z

@gpotter2 maybe we could have run_scapy{,.bat} that runs iPython with modified prompt to mimic current behavior?

gpotter2 · 2017-08-23T12:34:55Z

@p-l- Sounds good. Will work onto this.

gpotter2 · 2017-08-23T14:39:25Z

@p-l- @guedou I've updated the shell.
Could you please try it and give your feedback ? Thanks !

gpotter2 · 2017-08-25T09:22:43Z

Fixed the bug "No warning messages were shown", added indirectedly by @p-l- in https://github.com/secdev/scapy/pull/764/files.
I added a warning in the code (main.py) so this will be known in the future :)

guedou · 2017-08-25T11:17:25Z

There are some changes to the current behavior:

the prompt ">>>" is not white anymore
the shell asks "Do you really want to exit ([y]/n)?"
the history is not written to ~/.scapy_history

I think that they should be fixed.

gpotter2 · 2017-08-25T13:01:05Z

@guedou Fixed all of your comments !
If you are willing to change the colors, tell me. I do find the white a bit weird...

p-l- · 2017-08-25T14:03:20Z

Great job! One problem I have with this code for now: when I type: IP()/[tab], iPython tries to complete with files at the root of my file system (/bin/, etc.). I have to type IP() / [tab] (note the spaces), it works, so IP() / IC[tab] will complete in ICMP.

gpotter2 · 2017-08-25T16:11:38Z

Well I don't know how to fix this :/
I'll ask on the Ipython github page i guess...

p-l- · 2017-08-25T17:59:39Z

This patch will add the fields as a completion on packet objects. The clean way to do that is normally to create a Packet.__dir__(self) method, but I don't know how to add the fields to the "normal" result of dir().

diff --git a/scapy/main.py b/scapy/main.py
index 38d37ac..23c0d83 100644
--- a/scapy/main.py
+++ b/scapy/main.py
@@ -314,9 +314,15 @@ def interact(mydict=None,argv=None,mybanner=None,loglevel=20):
             IPYTHON=False
         
     if IPYTHON:
+        from IPython.terminal.embed import InteractiveShellEmbed
         from IPython.terminal.prompts import Prompts, Token
+        from IPython.utils.generics import complete_object
         from traitlets.config.loader import Config
-        from IPython.terminal.embed import InteractiveShellEmbed
+        from scapy.packet import Packet
+
+        @complete_object.when_type(Packet)
+        def complete_packet(obj, prev_completions):
+            return prev_completions + [fld.name for fld in obj.fields_desc]
 
         banner = the_banner % (conf.version) + " using IPython %s" % IPython.__version__
         cfg = Config()

gpotter2 · 2017-09-13T12:45:40Z

@p-l- @guedou Updated according to the new logo PR !
Also, colors are now available (on windows) without any requirement + conf.color... is now used on IPython.

I won't be able to fix IPython autocompletion for now (i will try to submit a patch on their side i guess), so this PR may be merged as so. Please review it :-)

guedou

Cool PR. I have some small comments.

guedou · 2017-09-15T07:08:50Z

scapy/autorun.py

@@ -58,7 +58,7 @@ def autorun_commands(cmds, my_globals=None, ignore_globals=None, verb=0):
                if cmd:
                    sys.stderr.write(sys.__dict__.get("ps2","... "))
                else:
-                    sys.stderr.write(str(sys.__dict__.get("ps1",ColorPrompt())))
+                    sys.stderr.write(str(sys.__dict__.get("ps1",sys.ps1)))


Please put a space after the comma.

guedou · 2017-09-15T07:13:00Z

scapy/config.py

-
-def _prompt_changer(attr,val):
-    prompt = conf.prompt
+def _prompt_changer(attr, val):


Could you add the following docstring:
""Change the current prompt theme"""

guedou · 2017-09-15T07:13:43Z

scapy/data.py

@@ -188,7 +188,7 @@ def load_manuf(filename):
    IP_PROTOS=load_protocols(os.environ["SystemRoot"]+"\system32\drivers\etc\protocol")
    TCP_SERVICES,UDP_SERVICES=load_services(os.environ["SystemRoot"] + "\system32\drivers\etc\services")
    # Default value, will be updated by arch.windows
-    MANUFDB = load_manuf(os.environ["ProgramFiles"] + "\\wireshark\\manuf")
+    MANUFDB = None


Why are you modifying this line ? This should be in a dedicated PR if really needed.

guedou · 2017-09-15T07:14:50Z

scapy/main.py


-from scapy.config import conf
+# Do not add any imports here that would trigger a warning messsage


You mean: 'never ever add imports' or 'never add imports before this line' ? I think that you should make this textt more explicit.

# Never add any global import, in main.py, that would trigger a warning messsage # before the console handlers gets added in interact()

guedou · 2017-09-15T07:17:08Z

scapy/main.py

@@ -58,13 +54,17 @@ def _probe_config_file(cf):
    else:
        return cf_path

-def _read_config_file(cf):
+def _read_config_file(cf, _globals=globals(), _locals=locals(), interactive=True):


Could you add a docstring and explain what the kw args do ?

guedou · 2017-09-15T07:19:55Z

scapy/main.py

@@ -182,25 +182,29 @@ def list_contrib(name=None):


 def save_session(fname=None, session=None, pickleProto=-1):


Could you add a docstring ?

For example, """save current Scapy session to the file specified in the fname arg"""

guedou · 2017-09-15T07:20:24Z

scapy/main.py

    conf.color_theme = DefaultTheme()
+    conf.interactive = True
+    if loglevel is not None:
+        conf.logLevel=loglevel


Add space around =

guedou · 2017-09-15T07:21:33Z

scapy/themes.py

-                return ct.prompt(config.conf.prompt)
-        except:
-            return self.__prompt
+def apply_ipython_color(shell):


Docstring ? =)

guedou · 2017-09-15T07:21:47Z

scapy/utils.py

@@ -35,7 +35,7 @@ def get_temp_file(keep=False, autoext=""):
    f = os.tempnam("","scapy")
    if not keep:
        conf.temp_files.append(f+autoext)
-    return f
+    return f+autoext


Add spaces around +

guedou · 2017-09-15T07:22:05Z

test/pipetool.uts

@@ -264,6 +264,8 @@ os.unlink("t2.pcap")
 = Test InjectSink and Inject3Sink
 ~ needs_root

+import mock


Is this needed in this PR ?

This PR fixes some useless imports to be shared. This was a forgotten import, and crashed because of that.

gpotter2 · 2017-09-19T19:00:43Z

Removed trailing line:
before
now

@p-l- It would be great if this could be merged in a near future, as it needs to replace Pyreadline/GnuReadline on the Python 3 port.

gpotter2 · 2017-09-19T19:39:07Z

test/regression.uts

@@ -1413,10 +1413,11 @@ a=Ether(str(Ether()/IPv6()/TCP()))
 a.type == 0x86dd

 = IPv6 Class binding with GRE - build
-str(IP()/GRE()/Ether()/IP()/GRE()/IPv6()) == b'E\x00\x00f\x00\x01\x00\x00@/|f\x7f\x00\x00\x01\x7f\x00\x00\x01\x00\x00eX\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x08\x00E\x00\x00@\x00\x01\x00\x00@/|\x8c\x7f\x00\x00\x01\x7f\x00\x00\x01\x00\x00\x86\xdd`\x00\x00\x00\x00\x00;@\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01'
+s = str(IP(src="127.0.0.1")/GRE()/Ether(dst="ff:ff:ff:ff:ff:ff", src="00:00:00:00:00:00")/IP()/GRE()/IPv6(src="::1"))


In some cases, IPv6 resolution could get called through this function, which is not available on travis/appveyor. This is revealed because the fix-net6 pr was combined with this one.

p-l-

OK for me @guedou

guedou · 2017-09-20T08:53:59Z

I am fine with @gpotter2 changes. This PR can be merged as soon as the OS X tests stop.

gpotter2 mentioned this pull request Jun 30, 2017

Please add more doc: asynchronous completion prompt-toolkit/python-prompt-toolkit#518

Closed

gpotter2 changed the title ~~[Console] Remove pyreadline/gnureadline~~ [Console/Python 3] Remove pyreadline/gnureadline Jun 30, 2017

gpotter2 force-pushed the new-autocompletion branch 2 times, most recently from b5a6d9e to 8789142 Compare June 30, 2017 17:44

gpotter2 force-pushed the new-autocompletion branch from 0b4897b to 4694529 Compare July 1, 2017 01:24

gpotter2 force-pushed the new-autocompletion branch from 4694529 to 3dcf0da Compare July 8, 2017 22:47

gpotter2 commented Jul 8, 2017

View reviewed changes

gpotter2 force-pushed the new-autocompletion branch from 9445ff5 to 4483401 Compare August 11, 2017 10:09

gpotter2 force-pushed the new-autocompletion branch from 4483401 to baf1bf0 Compare August 23, 2017 14:37

gpotter2 force-pushed the new-autocompletion branch from 900f2f5 to 5b7594c Compare August 25, 2017 09:20

gpotter2 force-pushed the new-autocompletion branch from 5b7594c to 4a29b0b Compare August 25, 2017 09:26

gpotter2 force-pushed the new-autocompletion branch from 4a29b0b to 5475c7d Compare August 25, 2017 12:59

gpotter2 force-pushed the new-autocompletion branch 9 times, most recently from 002467c to 98fb01a Compare September 13, 2017 12:12

guedou requested changes Sep 15, 2017

View reviewed changes

gpotter2 force-pushed the new-autocompletion branch from 98fb01a to 1376c8c Compare September 17, 2017 16:36

gpotter2 added 4 commits September 19, 2017 20:28

Fix Doc, readme, appveyor

068df36

Fix warning texts not being printed

9bd2ab2

Add legacy style + histfile

1f6eb98

Add packet fields auto-completion (thanks to @p-l-)

87133ad

gpotter2 force-pushed the new-autocompletion branch from 1376c8c to a7e76d3 Compare September 19, 2017 18:57

gpotter2 force-pushed the new-autocompletion branch 2 times, most recently from 5dc2189 to 2b04e2f Compare September 19, 2017 19:22

gpotter2 added 2 commits September 19, 2017 21:37

Pickle, session, interact tests

c380c0b

Remove trailing line

333bd2b

gpotter2 force-pushed the new-autocompletion branch from 2b04e2f to 333bd2b Compare September 19, 2017 19:37

gpotter2 commented Sep 19, 2017

View reviewed changes

p-l- approved these changes Sep 20, 2017

View reviewed changes

p-l- assigned guedou Sep 20, 2017

guedou approved these changes Sep 20, 2017

View reviewed changes

guedou merged commit 887a967 into secdev:master Sep 20, 2017

gpotter2 deleted the new-autocompletion branch September 23, 2017 14:55


		from scapy.config import conf
		# Do not add any imports here that would trigger a warning messsage

		@@ -182,25 +182,29 @@ def list_contrib(name=None):


		def save_session(fname=None, session=None, pickleProto=-1):

[Console/Python 3] Remove pyreadline/gnureadline #703

[Console/Python 3] Remove pyreadline/gnureadline #703

Conversation

gpotter2 commented Jun 30, 2017 • edited Loading

codecov-io commented Jun 30, 2017 • edited Loading

Codecov Report

gpotter2 commented Jul 8, 2017

gpotter2 Jul 8, 2017 • edited Loading

Choose a reason for hiding this comment

p-l- commented Aug 11, 2017

gpotter2 commented Aug 11, 2017 • edited Loading

p-l- commented Aug 11, 2017

gpotter2 commented Aug 18, 2017 • edited Loading

guedou commented Aug 19, 2017 via email

p-l- commented Aug 20, 2017

gpotter2 commented Aug 22, 2017

p-l- commented Aug 23, 2017

gpotter2 commented Aug 23, 2017

gpotter2 commented Aug 23, 2017 • edited Loading

gpotter2 commented Aug 25, 2017 • edited Loading

guedou commented Aug 25, 2017

gpotter2 commented Aug 25, 2017

p-l- commented Aug 25, 2017

gpotter2 commented Aug 25, 2017

p-l- commented Aug 25, 2017

gpotter2 commented Sep 13, 2017 • edited Loading

guedou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpotter2 Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpotter2 Sep 17, 2017 • edited Loading

Choose a reason for hiding this comment

gpotter2 commented Sep 19, 2017 • edited Loading

gpotter2 Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

p-l- left a comment

Choose a reason for hiding this comment

guedou commented Sep 20, 2017

gpotter2 commented Jun 30, 2017 •

edited

Loading

codecov-io commented Jun 30, 2017 •

edited

Loading

gpotter2 Jul 8, 2017 •

edited

Loading

gpotter2 commented Aug 11, 2017 •

edited

Loading

gpotter2 commented Aug 18, 2017 •

edited

Loading

gpotter2 commented Aug 23, 2017 •

edited

Loading

gpotter2 commented Aug 25, 2017 •

edited

Loading

gpotter2 commented Sep 13, 2017 •

edited

Loading

gpotter2 Sep 17, 2017 •

edited

Loading

gpotter2 Sep 17, 2017 •

edited

Loading

gpotter2 commented Sep 19, 2017 •

edited

Loading

gpotter2 Sep 19, 2017 •

edited

Loading