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.

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

Response Decoder .... not completed? #41

Open
a2800276 opened this issue Nov 16, 2024 · 3 comments
Open

Response Decoder .... not completed? #41

a2800276 opened this issue Nov 16, 2024 · 3 comments

Comments

@a2800276
Copy link

a2800276 commented Nov 16, 2024

I ran into some issues using the ResponseDecoder that give me the impression that it's not (never been) working and/or currently a work in progress. E.g. the test cases in response_test.go are all marked SKIP :)

Just wanted to confirm my suspicion before I change anything.

The response I am trying to decode contains a 'unsupported' group. Decoding this is not implemented, in fact the 0x05 "Unsupported Group" tag is not being taken into consideration. In my specific case this results in the next byte, a value tag being interpreted as the 1st byte of a length field which wreaks havoc.

Digging further leads me to believe (and I sincerely hope I am not stepping on anyone's toes here) that the Response Decoder is a hot mess that has not seen much use and is included more or less as a placeholder. Decode should try to match 0 or more groups (RFC 8010 Ch 3.1.1.), instead it just tries to match Attributes and then retroactively recognize and group the collected attributes (or something like that).

I've created a pull request (#42) that hopefully illustrated the issue and an additional test case that attempts to decode the actual Response that triggered my problems. I don't think it would be too much of a hassle to write a proper Decode function and I would be willing to do it, if you wish. But it would probably be easier to just throw out the previous function and reimplement from scratch, so I wanted to ask ahead if there's interest or if I am missing something obvious.

Here is my Response, the amended PR ilustrating the issue is here #42 .

0000   02 00 00 01 00 00 00 01 01 47 00 12 61 74 74 72   .........G..attr
0010   69 62 75 74 65 73 2d 63 68 61 72 73 65 74 00 05   ibutes-charset..
0020   75 74 66 2d 38 48 00 1b 61 74 74 72 69 62 75 74   utf-8H..attribut
0030   65 73 2d 6e 61 74 75 72 61 6c 2d 6c 61 6e 67 75   es-natural-langu
0040   61 67 65 00 05 65 6e 2d 75 73 45 00 0b 70 72 69   age..en-usE..pri
0050   6e 74 65 72 2d 75 72 69 00 28 69 70 70 3a 2f 2f   nter-uri.(ipp://
0060   6c 6f 63 61 6c 68 6f 73 74 2f 70 72 69 6e 74 65   localhost/printe
0070   72 73 2f 53 45 43 33 30 43 44 41 37 41 38 30 43   rs/SEC30CDA7A80C
0080   30 32 05 10 00 0c 70 72 69 6e 74 65 72 2d 74 79   02....printer-ty
              ^ start of unsupported group (0x05)                               
                 ^ value tag "unsupported" (0x10) 
                    ^ length of the group (0x000c)
                 ^ decoder is falsely interpreting 0x1000 as the length.
                 
0090   70 65 00 0c 70 72 69 6e 74 65 72 2d 74 79 70 65   pe..printer-type
00a0   10 00 0a 64 65 76 69 63 65 2d 75 72 69 00 0a 64   ...device-uri..d
00b0   65 76 69 63 65 2d 75 72 69 10 00 11 70 72 69 6e   evice-uri...prin
00c0   74 65 72 2d 69 73 2d 73 68 61 72 65 64 00 11 70   ter-is-shared..p
00d0   72 69 6e 74 65 72 2d 69 73 2d 73 68 61 72 65 64   rinter-is-shared
00e0   04 42 00 0c 70 72 69 6e 74 65 72 2d 6e 61 6d 65   .B..printer-name
00f0   00 0f 53 45 43 33 30 43 44 41 37 41 38 30 43 30   ..SEC30CDA7A80C0
0100   32 41 00 10 70 72 69 6e 74 65 72 2d 6c 6f 63 61   2A..printer-loca
0110   74 69 6f 6e 00 00 41 00 0c 70 72 69 6e 74 65 72   tion..A..printer
0120   2d 69 6e 66 6f 00 26 53 61 6d 73 75 6e 67 20 4d   -info.&Samsung M
0130   32 38 38 78 20 53 65 72 69 65 73 20 28 53 45 43   288x Series (SEC
0140   33 30 43 44 41 37 41 38 30 43 30 32 29 41 00 16   30CDA7A80C02)A..
0150   70 72 69 6e 74 65 72 2d 6d 61 6b 65 2d 61 6e 64   printer-make-and
0160   2d 6d 6f 64 65 6c 00 14 53 61 6d 73 75 6e 67 20   -model..Samsung
0170   4d 32 38 38 78 20 53 65 72 69 65 73 23 00 0d 70   M288x Series#..p
0180   72 69 6e 74 65 72 2d 73 74 61 74 65 00 04 00 00   rinter-state....
0190   00 03 41 00 15 70 72 69 6e 74 65 72 2d 73 74 61   ..A..printer-sta
01a0   74 65 2d 6d 65 73 73 61 67 65 00 19 50 72 69 6e   te-message..Prin
01b0   74 65 72 20 69 73 20 72 65 61 64 79 20 74 6f 20   ter is ready to
01c0   70 72 69 6e 74 44 00 15 70 72 69 6e 74 65 72 2d   printD..printer-
01d0   73 74 61 74 65 2d 72 65 61 73 6f 6e 73 00 04 6e   state-reasons..n
01e0   6f 6e 65 45 00 15 70 72 69 6e 74 65 72 2d 75 72   oneE..printer-ur
01f0   69 2d 73 75 70 70 6f 72 74 65 64 00 1f 69 70 70   i-supported..ipp
0200   3a 2f 2f 31 39 32 2e 31 36 38 2e 31 37 38 2e 31   ://192.168.178.1
0210   36 37 2f 69 70 70 2f 70 72 69 6e 74 45 00 00 00   67/ipp/printE...
0220   26 69 70 70 3a 2f 2f 53 45 43 33 30 43 44 41 37   &ipp://SEC30CDA7
0230   41 38 30 43 30 32 2e 6c 6f 63 61 6c 2e 2f 69 70   A80C02.local./ip
0240   70 2f 70 72 69 6e 74 03                           p/print.
@phin1x
Copy link
Owner

phin1x commented Nov 27, 2024

Hi @a2800276,

I know that part of the library is hacky and that the library itself is missing some ipp functions. It was originally written to orchestrate a farm of cups servers, so I only implemented what was needed at the time.
If you want to rework the request decode i would appreciate it.

@a2800276
Copy link
Author

a2800276 commented Dec 9, 2024

Hi @phin1x,

I updated the pullrequest to provide a fix for decoding Responses. My code would probably be helped with a lot more sample IPP messages, but I currently don't have any on hand :)

Anyway, let me know if you have any questions or if there's anything else I can do!

@phin1x
Copy link
Owner

phin1x commented Dec 9, 2024

Thanx. I will have a look into the code at the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants