Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:  ahf   |Sponsor:
--+

Comment (by nickm):

 merged the pr to master.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  closed
 Priority:  Medium|  Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:  fixed
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:  ahf   |Sponsor:
--+
Changes (by nickm):

 * status:  merge_ready => closed
 * resolution:   => fixed


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:  ahf   |Sponsor:
--+
Changes (by ahf):

 * status:  needs_review => merge_ready


Comment:

 The Rust nightly Travis builder fails with:

 {{{
 error: toolchain 'nightly' is not installed
 }}}

 But all the other Travis builders are green, and the code looks good, so I
 think this is safe to get in.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:  ahf   |Sponsor:
--+

Comment (by ahf):

 The way I read the `default()` implementation for `CStr` is that it is:

 {{{
 #[stable(feature = "cstr_default", since = "1.10.0")]
 impl<'a> Default for &'a CStr {
 fn default() -> &'a CStr {
 const SLICE: &'static [c_char] = &[0];
 unsafe { CStr::from_ptr(SLICE.as_ptr()) }
 }
 }
 }}}

 Which seems to be what we are doing too, which means that to me it seems
 sensible to use the `unwrap_or_default()` method.

 Opened a PR to see what Travis says:
 https://github.com/torproject/tor/pull/466

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:  ahf   |Sponsor:
--+
Changes (by dgoulet):

 * reviewer:   => ahf
 * milestone:   => Tor: 0.3.6.x-final


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  needs_revision => needs_review


Comment:

 I opened #28079.

 I don't have git access right now, so I can't run CI on this branch.
 Eventually, someone will open a pull request and review the change.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by cyberpunks):

 Replying to [comment:4 teor]:
 > But I still think we don't want to hide errors by substituting the empty
 string.

 Misuse is extremely unlikely to slip in since this is only used on string
 literals. But yeah, he ideal solution would be statically asserting at
 compile-time that the passed literal has no NUL bytes in it, so the only
 one is the byte being appended.

 But defaulting to an empty string(in a case that is basically impossible
 to get) is the intentional documented behavior of the macro ever since it
 was first merged in #25185. Improving on that seems like a separate
 ticket.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by teor):

 Replying to [comment:3 cyberpunks]:
 > Replying to [comment:2 teor]:
 > > keep the interior nul byte (like the original code)
 >
 > ? There's zero behavior change in this patch.

 You're right. I misread the unsafe block.

 But I still think we don't want to hide errors by substituting the empty
 string.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by cyberpunks):

 Replying to [comment:2 teor]:
 > keep the interior nul byte (like the original code)

 ? There's zero behavior change in this patch.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  new => needs_revision


Comment:

 from_bytes_with_nul returns an error if there is more than one nul byte in
 the string.

 In this case, I think we either want to keep the interior nul byte (like
 the original code) or fail.
 Returning an empty string could be a source of subtle bugs.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.3.4.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by cyberpunks):

 https://gitgud.io/onionk/tor/compare/master...rust-cstr-unsafe

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #28077 [Core Tor/Tor]: remove unsafe block from cstr! macro

[Tor Bug Tracker & Wiki]

#28077: remove unsafe block from cstr! macro
+--
 Reporter:  cyberpunks  |  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Component:  Core Tor/Tor
  Version:  Tor: 0.3.4.1-alpha  |   Severity:  Normal
 Keywords:  rust|  Actual Points:
Parent ID:  | Points:
 Reviewer:  |Sponsor:
+--
 {{{
  .unwrap_or(
 unsafe{
 ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"\0")
 }
 )
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs