search

marcelvanherk / Conquest-DICOM-Server

Conquest DICOM server, aiming for complete source code release
118 stars 48 forks source link

dicomweb json on metadata malforfated json Error #26

Open hakancicektr opened 2 years ago

hakancicektr commented 2 years ago

Hello i were testing 1.5.0c i found issue with dicomweb json on metadata there is problem with creating json response. json response is invalid. there some values lost and json format damaged. when i tried with 2 images study . its working (OHIF) but if image more than 2 images json is malformated. OHIF throw Sopclass error.

here is sample. "0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},"" malformated part "0020000E":{"vr":"UI","Value":["1.3.46.670589.11.72012.5.0.3812.2017031407141108960"]} ,"00200011":{"vr":"IS","Value":[101]}, "00200012":{"vr":"IS","Value":[1]} ,"00200013":{"vr":"IS","Value":[9]

looks like sprintf mixing while putting values. i can provide more samples.

i am using centos 7 i couldnt complie servertask due compile errors. i tried to dgate rather it still work fine.

Problem looks like on dgate.cpp

static int luaserialize(lua_State *L) function.

marcelvanherk commented 2 years ago

Hi can you share the images where it goes wrong? I assume you are using the released 150c

On Wed, 7 Sep 2022, 03:25 hakancicektr, @.***> wrote:

Hello i were testing 1.5.0c i found issue with dicomweb json on metadata there is problem with creating json response. json response is invalid. there some values lost and json format damaged. when i tried with 2 images study . its working (OHIF) but if image more than 2 images json is malformated. OHIF throw Sopclass error.

here is sample. "0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},"" malformated part

"0020000E":{"vr":"UI","Value":["1.3.46.670589.11.72012.5.0.3812.2017031407141108960"]} ,"00200011":{"vr":"IS","Value":[101]}, "00200012":{"vr":"IS","Value":[1]} ,"00200013":{"vr":"IS","Value":[9]

looks like sprintf mixing while putting values. i can provide more samples.

i am using centos 7 i couldnt complie servertask due compile errors. i tried to dgate rather it still work fine.

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJPY4KQXOEVAS6EHKQ3V474KBANCNFSM6AAAAAAQGK4VRA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

marcelvanherk commented 2 years ago

Also interested in servertask compile errors. Marcel

On Wed, 7 Sep 2022, 03:25 hakancicektr, @.***> wrote:

Hello i were testing 1.5.0c i found issue with dicomweb json on metadata there is problem with creating json response. json response is invalid. there some values lost and json format damaged. when i tried with 2 images study . its working (OHIF) but if image more than 2 images json is malformated. OHIF throw Sopclass error.

here is sample. "0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},"" malformated part

"0020000E":{"vr":"UI","Value":["1.3.46.670589.11.72012.5.0.3812.2017031407141108960"]} ,"00200011":{"vr":"IS","Value":[101]}, "00200012":{"vr":"IS","Value":[1]} ,"00200013":{"vr":"IS","Value":[9]

looks like sprintf mixing while putting values. i can provide more samples.

i am using centos 7 i couldnt complie servertask due compile errors. i tried to dgate rather it still work fine.

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJPY4KQXOEVAS6EHKQ3V474KBANCNFSM6AAAAAAQGK4VRA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

marcelvanherk commented 2 years ago

A quick code read shows nothing strange there (and code was tested on hundreds of studies in debian and windows). Can you give the steps to install on centos or create a Dockerfile ? I can then test it at home.

Marcel

On Wed, Sep 7, 2022 at 3:25 AM hakancicektr @.***> wrote:

Hello i were testing 1.5.0c i found issue with dicomweb json on metadata there is problem with creating json response. json response is invalid. there some values lost and json format damaged. when i tried with 2 images study . its working (OHIF) but if image more than 2 images json is malformated. OHIF throw Sopclass error.

here is sample. "0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},"" malformated part

"0020000E":{"vr":"UI","Value":["1.3.46.670589.11.72012.5.0.3812.2017031407141108960"]} ,"00200011":{"vr":"IS","Value":[101]}, "00200012":{"vr":"IS","Value":[1]} ,"00200013":{"vr":"IS","Value":[9]

looks like sprintf mixing while putting values. i can provide more samples.

i am using centos 7 i couldnt complie servertask due compile errors. i tried to dgate rather it still work fine.

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJPY4KQXOEVAS6EHKQ3V474KBANCNFSM6AAAAAAQGK4VRA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

hakancicektr commented 2 years ago

Hello,

in this part there " }, " between 1.2. and 000.7 is must not there .

"0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},""

should be correct one "0020000D":{"vr":"UI","Value":["1.2.000.7.572041167.76106034234"]},""

i sent you files . Patient Images and Metadata one correct metadata ( single image ) one incorrect (multi images)

i am using conquest linux 1417e with appox 1 million patients per month.

i would like to conturbate and share expriences.

If you interested you can contact me by +90 554 8848998 whatsapp or telegram (might better for fastest communication)

correct_json_metadata.txt malformat_json_metada.txt 63791833_12.zip

marcelvanherk commented 2 years ago

Hi,

the problem is definitively Centos specific, at least on windows the images load fine in OHIF. That does not mean it is not a bug on my side, such things may appear only with certain compilers. Can you share a Centos Dockerfile / install instructions so that I can make a test environment?

I any case the effect is that 4 characters text randomly overwrite the output, may be related to socket issues. Can you also try the node.js api?

Marcel

Marcel

On Wed, Sep 7, 2022 at 10:19 AM hakancicektr @.***> wrote:

Hello,

in this part there " }, " between 1.2. and 000.7 is must not there .

"0020000D":{"vr":"UI","Value":["1.2.},"000.7.572041167.76106034234"]},""

should be correct one "0020000D":{"vr":"UI","Value":["1.2.000.7.572041167.76106034234"]},""

i sent you files . Patient Images and Metadata one correct metadata ( single image ) one incorrect (multi images)

i am using conquest linux 1417e with appox 1 million patients per month.

i would like to conturbate and share expriences.

If you interested you can contact me by +90 554 8848998 whatsapp or telegram (might better for fastest communication)

correct_json_metadata.txt https://github.com/marcelvanherk/Conquest-DICOM-Server/files/9504642/correct_json_metadata.txt malformat_json_metada.txt https://github.com/marcelvanherk/Conquest-DICOM-Server/files/9504643/malformat_json_metada.txt 63791833_12.zip https://github.com/marcelvanherk/Conquest-DICOM-Server/files/9504655/63791833_12.zip

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1239131676, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJP7OLLKPUQSJ2EDSFTV5BM2BANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 2 years ago

Hi,

I think this is data corruption while transmitting from conquest to servertask. What compiler flags do you use?

Marcel

hakancicektr commented 2 years ago

its looks like problem with pointer usage there is result pointer but some codes writing over it indirectly while Vr->data formating its adding to result but there is index to where data will put. some how index moving backward and writing data to wrong position then is overwrite existing data. that is what am susspected. i am still analizing your codes.

marcelvanherk commented 2 years ago

Hi, I don't think so the same images and the same code work well on windows and debian.

Can you try this: ./dgate "--lua:x=DicomObject:new();x:Read(dicomfilename);f=io.open('/tmp/x.json', 'w'); f:write(x:Serialize(true, true, false)); f:close()"

This does the same serialisation but writes directly to file.

Or you can modify the rquery.lua function getmetadata to write a copy of the json data to file there (returnfile is deleted and is hard to get).

My guess is that the output of Serialize(true, true, false) is correct but that the issue is in socket communication. I had previous problems there.

Marcel

On Wed, 7 Sep 2022, 21:39 hakancicektr, @.***> wrote:

its looks like problem with pointer usage there is result pointer but some codes writing over it indirectly while Vr->data formating its adding to result but there is index to where data will put. some how index moving backward and writing data to wrong position then is overwrite existing data. that is what am susspected. i am still analizing your codes.

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1239850370, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJI4EWAZQUA3KSJLFWDV5D4RJANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 2 years ago

This issue appears in socket.cxx and buffer.cxx. You could roll them back to 1417d version to see what happens.

hakancicektr commented 2 years ago

i have changed default break size to oldvalue / 2 then all worked fine.

include "dicom.hpp"

ifdef UNIX

define DEFAULT_BREAK_SIZE 4096

else

define DEFAULT_BREAK_SIZE 16384

endif

hakancicektr commented 2 years ago

but still there is corruption on image even i treid smaller value image

marcelvanherk commented 2 years ago

Ah,

This is the same issue. I will send an update to socket.cxx tonight to try and fix it.

Edit: api/dicom uses a lot of internal tcp/ip communication and apparantly that is more sensitive for bugs in the socket layer. This is good, because I though they have been fixed, but apparently not. I think I will use luasockets code and link that to socket.cxx.

Marcel

marcelvanherk commented 2 years ago

Hi,

these new source files for in 1.5.0c integrate the socket layer from luasocket into the dicom server. They are still a bit rough, but they compile and run under windows and Debian. Can you try on your Centos system? Servertask does not compile with them but you can use dgate there. With them BREAK_SIZE should not matter anymore.

regards,

Marcel

On Thu, Sep 8, 2022 at 8:42 AM hakancicektr @.***> wrote:

but still there is corruption on image even i treid smaller value [image: image] https://user-images.githubusercontent.com/112996705/189064410-b1184120-a45b-4406-9ffd-5654991e7be8.png

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1240347757, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJLNEYYL7624BJF6QOLV5GKHJANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 2 years ago

ctest.zip

hakancicektr commented 2 years ago

Hello thank you for your efort

after complie dgate listener got error

Error in Accept() function call Attemping to re-bind socket

hakancicektr commented 2 years ago

if i use new complied as bridge (rather servertask) and on server side old one complied then server side

No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated

marcelvanherk commented 2 years ago

Ok, needs more work. Will do that tonight. Thanks for testing

On Fri, 9 Sep 2022, 02:02 hakancicektr, @.***> wrote:

if i use new complied as bridge (rather servertask) and on server side old one complied then server side

No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1241384483, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJIDILKW3QAKATW3SOLV5KEC7ANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 2 years ago

This will be errors in socket.cxx

On Fri, 9 Sep 2022, 02:02 hakancicektr, @.***> wrote:

if i use new complied as bridge (rather servertask) and on server side old one complied then server side

No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated No valid presentation contexts/transfer syntax found in 0 candidates In 0 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1241384483, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJIDILKW3QAKATW3SOLV5KEC7ANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

hakancicektr commented 2 years ago

Hi, i have some ideas and kind of request for improve conquest. i have to many working experiences with conquest. where i can share with you here new topic or by direct email ?

marcelvanherk commented 2 years ago

Happy to discuss in the forum.

On Fri, 9 Sep 2022, 13:32 hakancicektr, @.***> wrote:

Hi, i have some ideas and kind of request for improve conquest. i have to many working experiences with conquest. where i can share with you here new topic or by direct email ?

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1241921234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJIT657IJR7NHRCV223V5MU7NANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

hakancicektr commented 2 years ago

Okay. i did open thread devoloper section. i have allready appox 700 hospitals runing Teleradilogy services 500k~1m patients per months and i am using conquest at backend.

i might helpfull and happy improve conquest.

marcelvanherk commented 2 years ago

socket_test.zip

Hi again, can you try this socket version on centos? Absolutely not suitable for production but this on tests OK (now properly tested) on Debian and Windows (OHIF runs).

Curious if it helps with the json/image corruption issues. Expect a few more versions for testing.

marcelvanherk commented 2 years ago

The accept sockets times out after 5 minutes, so only use it for short tests.

hakancicektr commented 2 years ago

Hello, Better than before but still error in json and corrupted images.

hakancicektr commented 2 years ago

no no json is okay. error during load images only.

hakancicektr commented 2 years ago

No valid presentation contexts/transfer syntax found in 0 candidates In 1 presentation contexts #Possible transfer syntaxes: 13 multiplex: connection terminated No accepted presentation contexts/transfer syntax found in 1 candidates lua run error [string " local ae='01';..."]:5: attempt to index a nil value in ' local ae='01'; local level='STUDY'; local dicomweb=true; local q2=DicomObject:new(' {"00201208":"","00201206":"","00200010":"","0020000D":"","00100040":"","00100030":"","00100020":"","00100010":"","00080201":"","00081190":"","00080090":"","00080061":"","00080056":"","00080054":"","00080050":"","00080030":"","00080020":"","00080005":"","limit":"25","offset":"0","fuzzymatching":"false","includefield":"00081030,00080060","99990C01":"25","99990C02":"0"} '); local r = dicomquery(ae, level, q2):Serialize(true,false,dicomweb); local s=tempfile('.txt') local f=io.open(s, "wb") f:write(r) returnfile=s f:close(); '

hakancicektr commented 2 years ago

small json is okay. larger json serie with too many images still json error

marcelvanherk commented 2 years ago

Hm,

replaced entire socket implementation, bit out my depth. Can you try the buffer.cxx and socket.cxx from 1.4.14e in 1.5.0c?

Marcel

On Sat, Sep 10, 2022 at 8:45 PM hakancicektr @.***> wrote:

Hello, Better than before but still error in json and corrupted images.

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1242795075, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJL2KHH2S4N5T3Q4VZTV5TQNTANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

hakancicektr commented 2 years ago

okay i did same error like begin. directly json error.

hakancicektr commented 2 years ago

files from 1.4.17.e2

marcelvanherk commented 2 years ago

Hi,

can you roll back to 1.5.0c and add writing of a logfile to socket.cxx

I would suggest

socket# write N result R socket# read N result R

where N is the amount and R is the return code.

Add to writebinary and readbinary.

Thanks,

Marcel

On Sat, Sep 10, 2022 at 9:10 PM hakancicektr @.***> wrote:

files from 1.4.17.e2

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1242798056, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJK7YBLTRXOP425226LV5TTK7ANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 2 years ago

socket_logging.zip

Hi,

this is socket.cxx with a simple logging. It would be nice if this could help spot the error.

The output it produces looks like:

0140 reads 100 bytes sum 2164 0144 writes 100 bytes sum 2164 0140 reads 276 bytes sum 9307 0144 writes 276 bytes sum 9307 0144 writes 100 bytes sum 2164 0140 reads 100 bytes sum 2164 0140 reads 274 bytes sum 9245 0144 writes 274 bytes sum 9245 0144 writes 100 bytes sum 2164 0140 reads 100 bytes sum 2164 0144 writes 276 bytes sum 9287 0144 writes 100 bytes sum 2164 0140 reads 376 bytes sum 11451 0144 writes 274 bytes sum 9241 0144 writes 100 bytes sum 2164 0140 reads 374 bytes sum 11405

Can you try it out on Centos?

Marcel

hakancicektr commented 2 years ago

should i revert to all files to orginal version ? 1.5.0.c ?

marcelvanherk commented 2 years ago

Yes,

all files except the modified socket.cxx in the previous post. If you run it on both sides you get two socket.log files to compare. Delete these files before your test and then do minimal activity to show the error and then copy both files.

thanks

Marcel

On Tue, Sep 13, 2022 at 11:44 AM hakancicektr @.***> wrote:

should i revert to all files to orginal version ? 1.5.0.c ?

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1245230410, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJIKKUMOQLZVCVBZVVTV6BLKFANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

hakancicektr commented 2 years ago

I tested with windows but wado on linux (I put same patients on linux & windows)İ got  bugs on image like shifted pixels Data corruption so its looks like socket problem i will do recomplie with 1417d socket and buffer.cxx and test

Android’de Yahoo Postadan gönderildi

9:57’’8e’ 8 Eyl 2022 Per tarihinde, @.***> şunu yazdı:

This issue appears in socket.cxx and buffer.cxx. You could roll them back to 1417d version to see what happens.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

marcelvanherk commented 2 years ago

Hi, it is a socket issue that i have not been able to resolve yet. It does not appear on windows and later debian versions

On Tue, 11 Oct 2022, 10:16 hakancicektr, @.***> wrote:

I tested with windows but wado on linux (I put same patients on linux & windows)İ got bugs on image like shifted pixels Data corruption so its looks like socket problem i will do recomplie with 1417d socket and buffer.cxx and test

Android’de Yahoo Postadan gönderildi

9:57’’8e’ 8 Eyl 2022 Per tarihinde, @.***> şunu yazdı:

This issue appears in socket.cxx and buffer.cxx. You could roll them back to 1417d version to see what happens.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/marcelvanherk/Conquest-DICOM-Server/issues/26#issuecomment-1274378546, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVDWJIK53SLVMNZD4JDZ4LWCUV77ANCNFSM6AAAAAAQGK4VRA . You are receiving this because you commented.Message ID: @.***>

marcelvanherk commented 1 month ago

Hi PDUSize can be configured in dicom.ini in 1.5.0e. Try to see if this can be sorted by changing PDUsize. Have no solution otherwise.