#26592 closed defect (fixed)
py3: combinat/designs: bytes-like object is required, not 'str'
Reported by: | slabbe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.8 |
Component: | python3 | Keywords: | thursdaysbdx |
Cc: | dgordon, slabbe, slelievre, 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: 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'
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 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'
comment:3 Changed 4 years ago by
Feel free to repurpose the present ticket for that.
comment:4 Changed 4 years ago by
- Component changed from doctest coverage to python3
- Description modified (diff)
- Summary changed from internet doctest failing in combinat/designs [SSL: UNKNOWN_PROTOCOL] to py3: combinat/designs: bytes-like object is required, not 'str'
Ok, I updated the goal of this ticket as suggested.
comment:5 Changed 4 years ago by
- Milestone changed from sage-8.5 to sage-8.7
comment:6 Changed 3 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 3 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) <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 ?
comment:8 Changed 3 years ago by
- Cc slelievre 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 html-formatted string)
comment:9 Changed 3 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 3 years ago by
- Keywords thursdaysbdx added
comment:11 Changed 3 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 de-html-ify it.
comment:12 Changed 3 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 computer.
comment:13 Changed 3 years ago by
- Cc dgordon slabbe 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 3 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: 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?
comment:15 Changed 3 years ago by
- do not break the import line
- OR tag the first line of the import
- OR use optional=sage,internet ?
comment:16 Changed 3 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) <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__'
comment:17 Changed 3 years ago by
- Branch set to u/vklein/26592
comment:18 Changed 3 years ago by
- Commit set to 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 3 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 3 years ago by
- Cc zerline added
comment:21 Changed 3 years ago by
- Status changed from new to needs_info
comment:22 Changed 3 years ago by
- Milestone changed from sage-8.7 to sage-8.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 changed from u/vklein/26592 to u/slelievre/26592
comment:24 Changed 3 years ago by
- Commit changed from 27eeb10c43f0a090de107ebd5f427ad26de3bf53 to e863721bac6eca92b83d17a220692c127dffc88d
- Status changed from needs_info to 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 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.
comment:26 Changed 3 years ago by
- Commit changed from e863721bac6eca92b83d17a220692c127dffc88d to a69f4ecdf33625f037bbad1375c6b3f24a5b5f12
Branch pushed to git repo; I updated commit sha1. New commits:
a69f4ec | 26592: Move imports to beginning of file
|
comment:27 Changed 3 years ago by
Like this?
comment:28 follow-up: ↓ 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 in reply to: ↑ 28 ; follow-up: ↓ 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/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.
comment:30 Changed 3 years ago by
Or we could link to the copy archived at the Internet Archive's Wayback Machine:
comment:31 in reply to: ↑ 29 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 changed from a69f4ecdf33625f037bbad1375c6b3f24a5b5f12 to 1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1
Branch pushed to git repo; I updated commit sha1. New commits:
1f3afa8 | Update Gordon--Stinson bibliographic reference
|
comment:34 Changed 3 years ago by
Updated now, please review.
comment:35 Changed 3 years ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
ok, thx
comment:36 Changed 3 years ago by
- Branch changed from u/slelievre/26592 to 1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:37 Changed 3 years ago by
- Commit 1f3afa8ed60c5ab665a2d7ed3421323fe21dd6c1 deleted
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?
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)
comment:39 Changed 3 years ago by
Open a new ticket to fix that #27821
The whole https://math.ccrwest.org was down for a few days. It's back up now.