py3: combinat/designs: bytes-like object is required, not 'str'

[slabbe]

sage: print(version())
SageMath version 8.5.beta1, Release Date: 2018-10-27
sage: print(sys.version)
3.6.6 (default, Oct 27 2018, 01:34:38)
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)]
sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-a3d9f9e592f1> in <module>()
----> 1 C = designs.best_known_covering_design_from_LJCR(Integer(7), Integer(3), Integer(2))

/opt/s/sage/local/lib/python3.6/site-packages/sage/combinat/designs/covering_design.py in best_known_covering_design_www(v, k, t, verbose)
    505     f.close()
    506
--> 507     if 'covering not in database' in s:   #not found
    508         str = "no (%d,%d,%d) covering design in database\n"%(v,k,t)
    509         raise ValueError(str)

TypeError: a bytes-like object is required, not 'str'

[slelievre]

The whole ​https://math.ccrwest.org was down for a few days. It's back up now.

[slelievre]

This needs some Python 3 love now though.

Here's what I get with a Python3-based Sage:

sage: print(version())
SageMath version 8.5.beta1, Release Date: 2018-10-27
sage: print(sys.version)
3.6.6 (default, Oct 27 2018, 01:34:38)
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)]
sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-a3d9f9e592f1> in <module>()
----> 1 C = designs.best_known_covering_design_from_LJCR(Integer(7), Integer(3), Integer(2))

/opt/s/sage/local/lib/python3.6/site-packages/sage/combinat/designs/covering_design.py in best_known_covering_design_www(v, k, t, verbose)
    505     f.close()
    506
--> 507     if 'covering not in database' in s:   #not found
    508         str = "no (%d,%d,%d) covering design in database\n"%(v,k,t)
    509         raise ValueError(str)

TypeError: a bytes-like object is required, not 'str'

[slelievre]

Feel free to repurpose the present ticket for that.

[slabbe]

Ok, I updated the goal of this ticket as suggested.

[slabbe]

As suggested in ​https://trac.sagemath.org/ticket/26502#comment:8, the url address apparently changed to:

​​http://dmgordon.org/cover/

So we need to redo the changes that were made in #26502 in order to fix this issue.

[vklein]

The problem with dmgordon.org is that the server certificate chain is incomplete:

sage: from six.moves.urllib.request import urlopen
sage: url = "https://dmgordon.org/cover/"
sage: f = urlopen(url)
---------------------------------------------------------------------------
URLError                                  Traceback (most recent call last)
<ipython-input-6-152a0d5393b6> in <module>()
----> 1 f = urlopen(url)

...

/home/vklein/odk/sage/local/lib/python2.7/urllib2.pyc in do_open(self, http_class, req, **http_conn_args)
   1196         except socket.error, err: # XXX what error?
   1197             h.close()
-> 1198             raise URLError(err)
   1199         else:
   1200             try:

URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:726)>

Should we find a way to ignore ssl verification or to contact dmgordon.org owner ?

[slelievre]

I just emailed Dan Gordon to report the various issues we are trying to tackle here:

• http vs https
• get_cover.php seems gone from ​http://ljcr.dmgordon.org/cover/ (show_cover.php returns some html-formatted string)

[vklein]

Thank you Samuel.
For the http vs https part, note that urlopen("http://dmgordon.org/cover/get_cover.php") return the famous HTTP Error 404: Not Found.

[slelievre]

My attempt so far is using:

src/sage/combinat/designs/covering_design.py

@@ -497 +497 @@
 
     param = ("?v=%s&k=%s&t=%s"%(v,k,t))
 
-    url = "https://math.ccrwest.org/cover/get_cover.php"+param
+    url = "http://ljcr.dmgordon.org/cover/show_cover.php" + param
     if verbose:
         print("Looking up the bounds at %s" % url)
     f = urlopen(url)

which returns html instead of plain text, but I guess we could easily de-html-ify it.

[vklein]

It's feasible but calling get_cover.php will be far cleaner.
Another concern for me is that without https, given that the current code call sage_eval on the result, this code look like an open door to inject any (sage) code you want on a sage user computer.

[slelievre]

The following changes should suffice once get_cover.php is repaired.

(I think it currently gives an extra initial \n\n<html>\n<body>\n we don't expect.)

src/sage/combinat/designs/covering_design.py

@@ -21 +21 @@
 REFERENCES:
 
 .. [1] La Jolla Covering Repository,
-  https://math.ccrwest.org/cover.html
+  http://ljcr.dmgordon.org/cover.html
 
 .. [2] Coverings,
   Daniel Gordon and Douglas Stinson,
-  https://math.ccrwest.org/gordon/hcd.pdf
+  https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/view
   from the Handbook of Combinatorial Designs
 
 AUTHORS:
@@ -452 +452 @@
 
 def best_known_covering_design_www(v, k, t, verbose=False):
     r"""
-    Gives the best known `(v,k,t)` covering design, using the database
-    available at `<https://math.ccrwest.org/cover.html>`_
+    Return the best known `(v, k, t)` covering design from an online database.
+
+    This uses the La Jolla Covering Repository, a database
+    available at `<http://ljcr.dmgordon.org/cover.html>`_
 
     INPUT:
@@ -488 +490 @@
     """
     # import compatible with py2 and py3
     from six.moves.urllib.request import urlopen
-
+    from sage.cpython.string import bytes_to_str
     from sage.misc.sage_eval import sage_eval
 
     v = int(v)
@@ -497 +499 @@
 
     param = ("?v=%s&k=%s&t=%s"%(v,k,t))
 
-    url = "https://math.ccrwest.org/cover/get_cover.php"+param
+    url = "http://ljcr.dmgordon.org/cover/get_cover.php" + param
     if verbose:
         print("Looking up the bounds at %s" % url)
-    f = urlopen(url)
-    s = f.read()
-    f.close()
+    with urlopen(url) as f:
+        s = bytes_to_str(f.read())
 
     if 'covering not in database' in s:   #not found
         str = "no (%d,%d,%d) covering design in database\n"%(v,k,t)

I'll propose a branch if Dan Gordon confirms this about get_cover.php.

[slelievre]

Dan Gordon was able to get rid of the extra initial \n\n<html>\n<body>\n. We're almost there, but one thing puzzles me about sage -t.

With the following code in src/sage/combinat/designs/covering_design.py

def best_known_covering_design_www(v, k, t, verbose=False):
    r"""
    Return the best known `(v, k, t)` covering design from an online database.

    This uses the La Jolla Covering Repository, a database
    available at `<http://ljcr.dmgordon.org/cover.html>`_

    INPUT:

    - ``v`` -- integer, the size of the point set for the design

    - ``k`` -- integer, the number of points per block

    - ``t`` -- integer, the size of sets covered by the blocks

    - ``verbose`` -- bool (default: ``False``), print verbose message

    OUTPUT:

    A :class:`CoveringDesign` object representing the ``(v, k, t)``-covering
    design with smallest number of blocks available in the database.

    EXAMPLES::

        sage: from sage.combinat.designs.covering_design import (
        ....:     best_known_covering_design_www)          # optional - internet
        sage: C = best_known_covering_design_www(7, 3, 2)  # optional - internet
        sage: print(C)                                     # optional - internet
        C(7,3,2) = 7
        Method: lex covering
        Submitted on: 1996-12-01 00:00:00
        0  1  2
        0  3  4
        0  5  6
        1  3  5
        1  4  6
        2  3  6
        2  4  5

    A ValueError is raised if the ``(v, k, t)`` parameters are not
    found in the database.
    """
    # import compatible with py2 and py3
    from six.moves.urllib.request import urlopen
    from sage.cpython.string import bytes_to_str
    from sage.misc.sage_eval import sage_eval

    v = int(v)
    k = int(k)
    t = int(t)

    param = "?v=%s&k=%s&t=%s" % (v, k, t)

    url = "http://ljcr.dmgordon.org/cover/get_cover.php" + param
    if verbose:
        print("Looking up the bounds at %s" % url)
    with urlopen(url) as f:
        s = bytes_to_str(f.read())

    if 'covering not in database' in s:  # not found
        str = "no (%d, %d, %d) covering design in database\n" % (v, k, t)
        raise ValueError(str)

    return sage_eval(s)

the function now works manually, i.e. I can do

sage: from sage.combinat.designs.covering_design import (
....:     best_known_covering_design_www)
sage: C = best_known_covering_design_www(7, 3, 2)
sage: print(C)
C(7, 3, 2) = 7
Method: lex covering
Submitted on: 1996-12-01 00:00:00
0  1  2
0  3  4
0  5  6
1  3  5
1  4  6
2  3  6
2  4  5

but, for some reason I cannot figure out, sage -t gives a NameError saying name 'best_known_covering_design_www' is not defined:

$ sage -t --optional=internet src/sage/combinat/designs/covering_design.py
Running doctests with ID 2019-03-08-21-29-22-a6581694.
Git branch: t-26592-covering-designs-www
Using --optional=internet
Doctesting 1 file.
sage -t --warn-long 81.7 src/sage/combinat/designs/covering_design.py
**********************************************************************
File "src/sage/combinat/designs/covering_design.py", line 514, in sage.combinat.designs.covering_design.best_known_covering_design_www
Failed example:
    C = best_known_covering_design_www(7, 3, 2)  # optional - internet
Exception raised:
    Traceback (most recent call last):
      File "/opt/s/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/opt/s/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.designs.covering_design.best_known_covering_design_www[0]>", line 1, in <module>
        C = best_known_covering_design_www(Integer(7), Integer(3), Integer(2))  # optional - internet
    NameError: name 'best_known_covering_design_www' is not defined
**********************************************************************
File "src/sage/combinat/designs/covering_design.py", line 515, in sage.combinat.designs.covering_design.best_known_covering_design_www
Failed example:
    print(C)                                     # optional - internet
Exception raised:
    Traceback (most recent call last):
      File "/opt/s/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/opt/s/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.designs.covering_design.best_known_covering_design_www[1]>", line 1, in <module>
        print(C)                                     # optional - internet
    NameError: name 'C' is not defined
**********************************************************************
1 item had failures:
   2 of   3 in sage.combinat.designs.covering_design.best_known_covering_design_www
    [2 tests, 2 failures, 0.01 s]
----------------------------------------------------------------------
sage -t --warn-long 81.7 src/sage/combinat/designs/covering_design.py  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 0.1 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds

Can somebody enlighten me?

[chapoton]

• do not break the import line

• OR tag the first line of the import

• OR use optional=sage,internet ?

[vklein]

@slelievre with your patch and python 2 i obtain the following:

sage: best_known_covering_design_www(7, 3, 2)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-a930c6c24a1d> in <module>()
----> 1 best_known_covering_design_www(Integer(7), Integer(3), Integer(2))

<ipython-input-1-598f13fbcc94> in best_known_covering_design_www(v, k, t, verbose)
     55     if verbose:
     56         print("Looking up the bounds at %s" % url)
---> 57     with urlopen(url) as f:
     58         s = bytes_to_str(f.read())
     59 

AttributeError: addinfourl instance has no attribute '__exit__'

See: ​https://stackoverflow.com/questions/30627937/traceback-attributeerroraddinfourl-instance-has-no-attribute-exit/30628142

[vklein]

The py2 and py3 tests pass with branch u/vklein/26592.

I will now look for a way to avoid calling sage_eval on a internet given result.
It really looks like an highway for code injection to me.

---

New commits:

​27eeb10

Trac #26592: apply and fix slelievre's patch ...

[vklein]

Constructing "manually" the CoveringDesign is not that easy because his __init__ function has eleven named parameters with default values, that mean there is a lot of potential string format that get_cover.php could return.

Do we know if get_cover.php always return a string in the form "CoveringDesign?(<all init parameters positionally>)" ?

[embray]

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

[slelievre]

New branch based on Sage 8.8.beta2.

A first commit does pep8 reformatting + updating urls.

The second commit has the actual bytes_to_str fix in best_known_covering_design_www and a minor improvement to is_covering.

Please review.

---

New commits:

​a1437e5	26592: Fix urls and do a round of pep8 in covering_designs.py
​e863721	26592: Fix py3 bytes_to_str + minor speedup

[embray]

There is no point in these being in-line imports:

     from six.moves.urllib.request import urlopen
-
+    from sage.cpython.string import bytes_to_str

Those modules are always loaded by Sage so they might as well go at the module-level.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​a69f4ec

26592: Move imports to beginning of file

[slelievre]

Like this?

[chapoton]

This line:

https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/view

is not something that we should link to.

Otherwise, looks good. In the future, please avoid doing large scale pep8 cleanup at the same time as serious changes.

[slelievre]

Replying to chapoton:

This line:

https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/view

is not something that we should link to.

Other options I can see are the following:

• Dan Gordon's publications page: ​https://www.dmgordon.org/Publications/
• the CRC Press page for the book: ​https://www.crcpress.com/Handbook-of-Combinatorial-Designs-Second-Edition/Colbourn-Dinitz/p/book/9781584885061
• one of the editors' page for the book: ​http://www.cems.uvm.edu/~jdinitz/hcd.html

Otherwise, looks good. In the future, please avoid doing large scale pep8 cleanup at the same time as serious changes.

Sorry about that, I got slightly carried away. I tried to use a different commit for each type of changes, but probably could have done a better job of that, or used different tickets altogether. Thanks for reviewing anyway and for the advice for the future.

[slelievre]

Or we could link to the copy archived at the Internet Archive's Wayback Machine:

• ​https://web.archive.org/web/20170829210149/https://www.ccrwest.org/gordon/hcd.pdf

[chapoton]

Replying to slelievre:

Replying to chapoton:

This line:

https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/view

is not something that we should link to.

Other options I can see are the following:

• Dan Gordon's publications page: ​https://www.dmgordon.org/Publications/

Yes, this one is better. One should also give the title (Coverings, chapter 1 of whatever)

[dgordon]

Having my papers on google drive was the last part of my website that I hadn't updated. I've moved them now, so that paper can now be linked to at ​https://www.dmgordon.org/papers/hcd.pdf.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​1f3afa8

Update Gordon--Stinson bibliographic reference

[slelievre]

Updated now, please review.

[chapoton]

ok, thx

[slabbe]

With 8.8.beta5, I get:

sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-a3d9f9e592f1> in <module>()
----> 1 C = designs.best_known_covering_design_from_LJCR(Integer(7), Integer(3), Integer(2))

/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/sage/combinat/designs/covering_design.pyc in best_known_covering_design_www(v, k, t, verbose)
    525     if verbose:
    526         print("Looking up the bounds at %s" % url)
--> 527     with urlopen(url) as f:
    528         s = bytes_to_str(f.read())
    529     if 'covering not in database' in s:  # not found

AttributeError: addinfourl instance has no attribute '__exit__'

Were you obtaining the same issue with this branch?

[vklein]

I think so with urelopen(url) as... syntax has been introduced with commit e863721.

It should be avoided (see ​comment:16)

[vklein]

Open a new ticket to fix that #27821