#26592 closed defect (fixed)
py3: combinat/designs: byteslike object is required, not 'str'
Reported by:  Sébastien Labbé  Owned by:  

Priority:  major  Milestone:  sage8.8 
Component:  python3  Keywords:  thursdaysbdx 
Cc:  Dan Gordon, Sébastien Labbé, Samuel Lelièvre, vklein, zerline  Merged in:  
Authors:  Samuel Lelièvre  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  1f3afa8 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
sage: print(version()) SageMath version 8.5.beta1, Release Date: 20181027 sage: print(sys.version) 3.6.6 (default, Oct 27 2018, 01:34:38) [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang700.1.81)] sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)  TypeError Traceback (most recent call last) <ipythoninput1a3d9f9e592f1> in <module>() > 1 C = designs.best_known_covering_design_from_LJCR(Integer(7), Integer(3), Integer(2)) /opt/s/sage/local/lib/python3.6/sitepackages/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 byteslike object is required, not 'str'
Change History (39)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
This needs some Python 3 love now though.
Here's what I get with a Python3based Sage:
sage: print(version()) SageMath version 8.5.beta1, Release Date: 20181027 sage: print(sys.version) 3.6.6 (default, Oct 27 2018, 01:34:38) [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang700.1.81)] sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)  TypeError Traceback (most recent call last) <ipythoninput1a3d9f9e592f1> in <module>() > 1 C = designs.best_known_covering_design_from_LJCR(Integer(7), Integer(3), Integer(2)) /opt/s/sage/local/lib/python3.6/sitepackages/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 byteslike object is required, not 'str'
comment:4 Changed 4 years ago by
Component:  doctest coverage → python3 

Description:  modified (diff) 
Summary:  internet doctest failing in combinat/designs [SSL: UNKNOWN_PROTOCOL] → py3: combinat/designs: byteslike object is required, not 'str' 
Ok, I updated the goal of this ticket as suggested.
comment:5 Changed 4 years ago by
Milestone:  sage8.5 → sage8.7 

comment:6 Changed 4 years ago by
As suggested in https://trac.sagemath.org/ticket/26502#comment:8, the url address apparently changed to:
So we need to redo the changes that were made in #26502 in order to fix this issue.
comment:7 Changed 4 years ago by
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) <ipythoninput6152a0d5393b6> 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 ?
comment:8 Changed 4 years ago by
Cc:  Samuel Lelièvre added 

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 htmlformatted string)
comment:9 Changed 4 years ago by
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
.
comment:10 Changed 4 years ago by
Keywords:  thursdaysbdx added 

comment:11 Changed 4 years ago by
My attempt so far is using:

src/sage/combinat/designs/covering_design.py
diff git a/src/sage/combinat/designs/covering_design.py b/src/sage/combinat/designs/covering_design.py index 87dda39064..23beebc2e8 100644
a b def best_known_covering_design_www(v, k, t, verbose=False): 497 497 498 498 param = ("?v=%s&k=%s&t=%s"%(v,k,t)) 499 499 500 url = "http s://math.ccrwest.org/cover/get_cover.php"+param500 url = "http://ljcr.dmgordon.org/cover/show_cover.php" + param 501 501 if verbose: 502 502 print("Looking up the bounds at %s" % url) 503 503 f = urlopen(url)
which returns html instead of plain text, but I guess we could easily dehtmlify it.
comment:12 Changed 4 years ago by
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's computer.
comment:13 Changed 4 years ago by
Cc:  Dan Gordon Sébastien Labbé vklein added 

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
diff git a/src/sage/combinat/designs/covering_design.py b/src/sage/combinat/designs/covering_design.py index 87dda39064..ef7512b704 100644
a b design from this database, we include extra information: 21 21 REFERENCES: 22 22 23 23 .. [1] La Jolla Covering Repository, 24 http s://math.ccrwest.org/cover.html24 http://ljcr.dmgordon.org/cover.html 25 25 26 26 .. [2] Coverings, 27 27 Daniel Gordon and Douglas Stinson, 28 https:// math.ccrwest.org/gordon/hcd.pdf28 https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/view 29 29 from the Handbook of Combinatorial Designs 30 30 31 31 AUTHORS: … … class CoveringDesign(SageObject): (this hunk was shorter than expected) 452 452 453 453 def best_known_covering_design_www(v, k, t, verbose=False): 454 454 r""" 455 Gives the best known `(v,k,t)` covering design, using the database 456 available at `<https://math.ccrwest.org/cover.html>`_ 455 Return the best known `(v, k, t)` covering design from an online database. 456 457 This uses the La Jolla Covering Repository, a database 458 available at `<http://ljcr.dmgordon.org/cover.html>`_ 457 459 458 460 INPUT: … … def best_known_covering_design_www(v, k, t, verbose=False): 488 490 """ 489 491 # import compatible with py2 and py3 490 492 from six.moves.urllib.request import urlopen 491 493 from sage.cpython.string import bytes_to_str 492 494 from sage.misc.sage_eval import sage_eval 493 495 494 496 v = int(v) … … def best_known_covering_design_www(v, k, t, verbose=False): 497 499 498 500 param = ("?v=%s&k=%s&t=%s"%(v,k,t)) 499 501 500 url = "http s://math.ccrwest.org/cover/get_cover.php"+param502 url = "http://ljcr.dmgordon.org/cover/get_cover.php" + param 501 503 if verbose: 502 504 print("Looking up the bounds at %s" % url) 503 f = urlopen(url) 504 s = f.read() 505 f.close() 505 with urlopen(url) as f: 506 s = bytes_to_str(f.read()) 506 507 507 508 if 'covering not in database' in s: #not found 508 509 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
.
comment:14 Changed 4 years ago by
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: 19961201 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: 19961201 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 20190308212922a6581694. Git branch: t26592coveringdesignswww Using optional=internet Doctesting 1 file. sage t warnlong 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/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/opt/s/sage/local/lib/python3.6/sitepackages/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/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/opt/s/sage/local/lib/python3.6/sitepackages/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 warnlong 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?
comment:15 Changed 4 years ago by
do not break the import line
comment:16 Changed 4 years ago by
@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) <ipythoninput2a930c6c24a1d> in <module>() > 1 best_known_covering_design_www(Integer(7), Integer(3), Integer(2)) <ipythoninput1598f13fbcc94> 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__'
comment:17 Changed 4 years ago by
Branch:  → u/vklein/26592 

comment:18 Changed 4 years ago by
Commit:  → 27eeb10c43f0a090de107ebd5f427ad26de3bf53 

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 ...

comment:19 Changed 4 years ago by
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>)" ?
comment:20 Changed 4 years ago by
Cc:  zerline added 

comment:21 Changed 4 years ago by
Status:  new → needs_info 

comment:22 Changed 4 years ago by
Milestone:  sage8.7 → sage8.8 

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:23 Changed 3 years ago by
Branch:  u/vklein/26592 → u/slelievre/26592 

comment:24 Changed 3 years ago by
Commit:  27eeb10c43f0a090de107ebd5f427ad26de3bf53 → e863721bac6eca92b83d17a220692c127dffc88d 

Status:  needs_info → needs_review 
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

comment:25 Changed 3 years ago by
There is no point in these being inline 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 modulelevel.
comment:26 Changed 3 years ago by
Commit:  e863721bac6eca92b83d17a220692c127dffc88d → a69f4ecdf33625f037bbad1375c6b3f24a5b5f12 

Branch pushed to git repo; I updated commit sha1. New commits:
a69f4ec  26592: Move imports to beginning of file

comment:28 followup: 29 Changed 3 years ago by
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.
comment:29 followup: 31 Changed 3 years ago by
Replying to chapoton:
This line:
https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/viewis 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/HandbookofCombinatorialDesignsSecondEdition/ColbournDinitz/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.
comment:30 Changed 3 years ago by
Or we could link to the copy archived at the Internet Archive's Wayback Machine:
comment:31 Changed 3 years ago by
Replying to slelievre:
Replying to chapoton:
This line:
https://drive.google.com/file/d/0B6s9_JNV1EjtUFd4MkhBc0hSdVU/viewis 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)
comment:32 Changed 3 years ago by
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.
comment:33 Changed 3 years ago by
Commit:  a69f4ecdf33625f037bbad1375c6b3f24a5b5f12 → 1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1 

Branch pushed to git repo; I updated commit sha1. New commits:
1f3afa8  Update GordonStinson bibliographic reference

comment:35 Changed 3 years ago by
Authors:  → Samuel Lelièvre 

Reviewers:  → Frédéric Chapoton 
Status:  needs_review → positive_review 
ok, thx
comment:36 Changed 3 years ago by
Branch:  u/slelievre/26592 → 1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:37 Changed 3 years ago by
Commit:  1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1 

With 8.8.beta5, I get:
sage: C = designs.best_known_covering_design_from_LJCR(7, 3, 2)  AttributeError Traceback (most recent call last) <ipythoninput1a3d9f9e592f1> 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/sitepackages/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?
comment:38 Changed 3 years ago by
I think so with urelopen(url) as...
syntax has been introduced with commit e863721
.
It should be avoided (see comment:16)
The whole https://math.ccrwest.org was down for a few days. It's back up now.