Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parseint_preamble bounds check fails for AbstractString #29451

Closed
samoconnor opened this issue Oct 1, 2018 · 1 comment
Closed

parseint_preamble bounds check fails for AbstractString #29451

samoconnor opened this issue Oct 1, 2018 · 1 comment
Labels
bug Indicates an unexpected problem or unintended behavior strings "Strings!"

Comments

@samoconnor
Copy link
Contributor

i <= ncodeunits(s) should be i <= endpos ?

julia/base/parse.jl

Lines 75 to 76 in 1a1d6b6

if c == '0' && i <= ncodeunits(s)
c, i = iterate(s,i)::Tuple{Char, Int}

When I do parse(Int, "0") where "0" is an AbstractString and ncodeunits > lastindex, i get this:
TypeError: in parseint_preamble, in typeassert, expected Tuple{Char,Int64}, got Nothing

1a1d6b6#r30713661

Monkeypatching parseint_preamble for my string type with i <= endpos seems to fix the problem.
samoconnor referenced this issue Oct 1, 2018
@Keno
Change iteration protocol
1a1d6b6 
This changes the iteration protocol from `start`/`next`/`done` to `iterate`.
The new lowering of a for loop is as follows:

```
for x in itr
    ...
end
```

becomes

```
next = iterate(itr)
while next !== nothing
    x, state = next::Tuple{Any, Any}
    ...
    next = iterate(itr, state)
end
```

The semantics are as apparent from the above lowering. `iterate` returns
either `nothing` or a tuple of value and state. The state is passed
to any subsequent operation. The first iteration is indicated, by not passing
the second, state argument to the `iterate` method.

Adaptors in both directions are provided to keep the legacy iteration
protocol working for now. However, performance of the legacy iteration
protocol will be severely pessimized.

As an optional add-on for mutable iterators, a new `isdone` function is
provided. This function is intended as an O(1) approximate query for
iterator completion, where such a calculation is possible without mutation
and/or is significantly faster than attempting to obtain the element itself.
The function makes use of 3-value logic. `missing` is always an acceptable
answer, in which case the caller should go ahead and attempt the iteration
to obtain a definite result. If the result is not `missing`, it must be
exact (i.e. if true, the next call to iterate must return `nothing`, if
false it must not return nothing).
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Integrate LazyHTTP with Messages.jl
094620f 
 - Replace ::Headers with ::LazyHTTP.Header in Message structs
 - Remove status, version, method and target fields
   (now lazily parsed from the LazyHTTP.Header)
 - Add getproperty methods for status, version, method and target
   fields.
 - Monkeypatch for JuliaLang/julia#29451
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Integrate LazyHTTP with Messages.jl
c355a73 
 - Replace ::Headers with ::LazyHTTP.Header in Message structs
 - Remove status, version, method and target fields
   (now lazily parsed from the LazyHTTP.Header)
 - Add getproperty methods for status, version, method and target
   fields.
 - Monkeypatch for JuliaLang/julia#29451
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Integrate LazyHTTP with Messages.jl
1905d7a 
 - Replace ::Headers with ::LazyHTTP.Header in Message structs
 - Remove status, version, method and target fields
   (now lazily parsed from the LazyHTTP.Header)
 - Add getproperty methods for status, version, method and target
   fields.
 - Monkeypatch for JuliaLang/julia#29451
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Integrate LazyHTTP with Messages.jl
3ed5e0d 
 - Replace ::Headers with ::LazyHTTP.Header in Message structs
 - Remove status, version, method and target fields
   (now lazily parsed from the LazyHTTP.Header)
 - Add getproperty methods for status, version, method and target
   fields.
 - Monkeypatch for JuliaLang/julia#29451
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Monkeypatch for JuliaLang/julia#29451
c514504 
Make AWS signing code lazy header compatible
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Monkeypatch for JuliaLang/julia#29451
e46027d 
Make AWS signing code lazy header compatible
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 1, 2018
@samoconnor
Monkeypatch for JuliaLang/julia#29451
8f83255 
Make AWS signing code lazy header compatible
@StefanKarpinski StefanKarpinski added bug Indicates an unexpected problem or unintended behavior strings "Strings!" labels Oct 1, 2018
@StefanKarpinski
Copy link
Sponsor Member

Since there seems to be a fix in that monkey patch, would you be willing to make a PR to fix this in Base Julia?

samoconnor added a commit to samoconnor/julia that referenced this issue Oct 2, 2018
@samoconnor
Fix JuliaLang#29451
14d7be0 
Needs a sanity check from someone who knows the detail of the parser code @Keno? @StefanKarpinski?
@samoconnor samoconnor mentioned this issue Oct 2, 2018
Merged
samoconnor added a commit to samoconnor/julia that referenced this issue Oct 2, 2018
@samoconnor
Issue JuliaLang#29451 test
ad63b1b
Keno added a commit that referenced this issue Oct 3, 2018
@Keno
Revert "Fix #29451: parse(Int, s::AbstractString) when ncodeunits(s) …
6482163 
…> lastindex(s) (#29465)"

This reverts commit a984ad0.
KristofferC pushed a commit that referenced this issue Oct 6, 2018
@samoconnor @KristofferC
Fix #29451: parse(Int, s::AbstractString) when ncodeunits(s) > lastin…
3cfe9ce 
…dex(s) (#29465)

(cherry picked from commit a984ad0)
@KristofferC KristofferC mentioned this issue Oct 6, 2018
Merged
samoconnor added a commit to JuliaWeb/HTTP.jl that referenced this issue Oct 16, 2018
@samoconnor
Monkeypatch for JuliaLang/julia#29451
46746b9 
Make AWS signing code lazy header compatible
KristofferC pushed a commit that referenced this issue Feb 11, 2019
@samoconnor @KristofferC
Fix #29451: parse(Int, s::AbstractString) when ncodeunits(s) > lastin…
f56d83a 
…dex(s) (#29465)

(cherry picked from commit a984ad0)
KristofferC pushed a commit that referenced this issue Feb 20, 2020
@samoconnor @KristofferC
Fix #29451: parse(Int, s::AbstractString) when ncodeunits(s) > lastin…
a9dcb6a 
…dex(s) (#29465)

(cherry picked from commit a984ad0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior strings "Strings!"
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@StefanKarpinski @samoconnor