QUIC-Tracker / quic-tracker

A test suite for QUIC
https://quic-tracker.info.ucl.ac.be
GNU Affero General Public License v3.0
75 stars 16 forks source link

Interface Conversion Error #20

Closed piano-man closed 4 years ago

piano-man commented 4 years ago

The error I'm running into is this :-

*_panic: interface conversion: quictracker.Frame is quictracker.MaxDataFrame, not quictracker.MaxDataFrame_**

To provide context, I have a function which receives a pointer to a frame i.e. *frame Frame.** I then determine the type of the frame using *_(frame).FrameType()_ which evaluates to MaxDataType. Following this, when I try to extract the MaximumData value using (frame).(MaxDataFrame).MaximumData**, I run into the error above.

I'm unable to figure out the correct way of extracting MaximumData value from the frame.

mpiraux commented 4 years ago

I always tried to implement and use Frame as a pointer type, so that you can either always do (*TheFrameType). Looks like I failed somewhere for MaxDataFrame. I will look at where this is coming from.

mpiraux commented 4 years ago

So the code in this repository seems to be type coherent. It may be coming from your code then ? I guess you instantiate a Frame with the value of MaxDataFrame struct and not a pointer to that value. The cast you mention is correct, but it only works when all your code agree on using frames as pointers. I don't use pointers of Frame, because all frames implementing the interface are already pointers. Go kind of restrain you from doing so by forcing you to type (*frame) before the cast. So, I can advice you to look at all frame structure initialisation and make sure you are passing a pointer to the struct to the other Frame handling parts the code.

piano-man commented 4 years ago

But if there was an issue with the initialization, wouldn't (*frame).FrameType() not work either ? Because I'm only going to (frame).(MaxDataFrame).MaximumData if (*frame).FrameType() == MaxDataType

piano-man commented 4 years ago

I essentially have a function that receives a pointer to a frame *_(frame Frame)_ and and then a switch case depending on the type of the frame. It works for all other frames, just throws this error for MaxStreamDataType, MaxDataType and MaxStreamsType**.

Also, I'm not really creating or initialising a frame. I pass a pointer to a packet *_(packet Packet)_** to a function from EncodeAndEncrypt. I then extract the frames from the packet using -

frames := (*packet).(Framer).GetFrames()
for i, _ := range frames {
            &((*packet).(Framer).GetFrames()[i]) // this is the pointer to the frame (frame *Frame) mentioned originally in the issue which I pass to the function 
    }
mpiraux commented 4 years ago

(*frame).FrameType() works because Go can cope with either *MaxDataFrame and MaxDataFrame for method invocation. But a direct cast is stricter. I am no Go expert so there might be plenty of better docs on that subject. By greping your code I found:

./scenarii/flow_control.go:
conn.FrameQueue.Submit(qt.QueuedFrame{qt.MaxDataFrame{uint64(conn.TLSTPHandler.MaxData)}, qt.EncryptionLevel1RTT})

I have fixed this in https://github.com/QUIC-Tracker/quic-tracker/commit/aa881bafef719b3f53a70b60b86547dd01eef633.

piano-man commented 4 years ago

I updated the code, but still run into the same problem.

I experimented a little more and found something really surprising. For the scenario where I was getting the above error, I changed (frame).(MaxDataFrame).MaximumData to (*frame).(MaxDataFrame).MaximumData and I was able to access the value(not change it but just read it). However, for a different scenario, I now get the following error - panic: interface conversion: quictracker.Frame is *quictracker.MaxDataFrame, not quictracker.MaxDataFrame which is a total opposite of the earlier error.

I figured maybe passing packets and frames using pointers is leading to the unexpected behaviour, so I removed all reference passing from my code and still got the same behaviour.

Considering that I have done no initialization and am just trying to access these values from packets when they reach EncodeAndEncrypt function, I can't understand what's happening at all.(Infact, just to be extra sure, I cloned a fresh copy of the repo and added only a couple of lines in the connection.go file which extract the frames from the packet to make sure something else in my code wasn't leading to this) Also, the problem occurs with different types of frames as well. And for a scenario, in the same packet, some types of frame require one type of cast while others require a different type. (I'm sorry if this is taking up too much of your time)

piano-man commented 4 years ago

Infact, this specific test case might help you see the error first hand. First, I add these lines to the EncodeAndEncrypt function in connection.go

if packet.PNSpace() == PNSpaceAppData {
        framePacket, ok := (packet).(Framer)                                                                                                                                       
        if ok {                                                                                                                                                                  
           frames := framePacket.GetFrames()                                                                                                                                         
           for _, frame := range frames {                                                                                                                                             
                fmt.Println("Frame type", frame.FrameType())                                                                                                                               
                if frame.FrameType() == StreamType {                                                                                                                                        
                     fmt.Println("Processing Stream Frame")                                                                                                                                                  
                      fmt.Println(frame.(*StreamFrame).StreamId)                                                                                                                                                                                                                                                                                                           
                }                                                                                                                                                                          
             }
          }                                                                                                                                                                         
}

Now, I run the scenario_runner.go script for _http3get and the _sever_flowcontrol scenario against the host quic.tech:8443. While http3_get will work fine, the server_flow_control scenario will throw the error - panic: interface conversion: quictracker.Frame is quictracker.StreamFrame, not *quictracker.StreamFrame. If I change frame.(*StreamFrame).StreamId to frame.(StreamFrame).StreamId, server_flow_control works fine while http3_get will throw the error - *_panic: interface conversion: quictracker.Frame is quictracker.StreamFrame, not quictracker.StreamFrame._**

mpiraux commented 4 years ago

Thank you for taking the time to dig this up. I rarely have to look into the packets that are sent and I have found some occurences of this type inconsistency recently when adding qlog. I found another one in server_flow_control. A fix is pushed that changes all Frame-implementing structs to use pointers. Now it fails to compile when using an existing frame struct as a value and not a pointer. I think this will end all the troubles listed here.

piano-man commented 4 years ago

Great ! Works well now :D