search

microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

Method visibility in VSCode #5191

Closed ghost closed 4 years ago

ghost commented 5 years ago

Dear Development Team,

our developer found this bug during writing extensions, seems very serious to me. Can you please investigate and let us me know?

The procedure GetCutomerSellToFaxNo on table “Sales Invoice Header” is not set to external like the surrounding procedures.

image

However, when used from AL, it works fine without parenthesis (compiles and executes) when used in the context of a page or report as the default record:

image

However adding brackets (as per CodeCop above) causes the following error:

image

To summarise, we can call this non-external procedure as long as we don’t add parenthesis as long as it is the default record, which doesn’t seem to be intended.

Trying to reproduce this with my own procedure, I added the following to the customer table in C/AL and added it to my extension (after generating and redownloading symbols):

image

image

My own procedure was rejected properly regardless of whether it has parenthesis.

I then rewrote the page extension to be based upon Customer List instead of Posted Sales Invoices.

image

I then confirmed that it does indeed work:

image

Trying to find out how exploitable this is, I tested the following bit of code:

image

As you can see above, the version of the message statement enclosed by “with SalesInvoiceHeader do” compiles with a warning about parenthesis as we’ve already seen, the version outside the with block fails since the procedure isn’t external.

With the second message remarked out, the code executes just fine and reports the customer number and customer fax number of the last invoice, despite the GetSellToCustomerFaxNo not being external.

As before, adding parenthesis shows that the 3 procedures are not external:

image

In fact you can abuse this pretty much anywhere you want as long as you don’t explicitly state the record and use a with block and don’t add parenthesis.

image

AlexanderYakunin commented 5 years ago

Thank you, we will investigate this asap.

AlexanderYakunin commented 5 years ago

GetCutomerSellToFaxNo is external in master, remaining issue is under investigation.

thpeder commented 5 years ago

Thanks for reporting the issue! I have reproduced the issue and it is indeed a bug.

ghost commented 5 years ago

Cool, thanks for letting me know.

yrest commented 5 years ago

I presume this is 14 Marton?

ghost commented 5 years ago

I presume this is 14 Marton?

yes, we've found this on the 25th of July...we haven't tested in V15 though Do you want us the test this in V15 @thpeder ?

thpeder commented 5 years ago

No thanks it is fine @martonnagy, I tested it in v15 and the fix will be coming to a future release of 15.

ALGitHubBot commented 4 years ago

The fix for this issue has been checked in to the master branch. It will be available in the bcinsider.azurecr.io/bcsandbox-master Docker image starting from platform build number 37837.

If you don’t have access to these images you need to become part of the Ready2Go program: aka.ms/readytogo

For more details on code branches and docker images please read: https://blogs.msdn.microsoft.com/nav/2018/05/03/al-developer-previews-multiple-releases-and-github/ https://blogs.msdn.microsoft.com/freddyk/2018/04/16/which-docker-image-is-the-right-for-you/